Good design to expose debug info from kernel module

[Elazar Leibovich]

And this is exactly my debate.

On the one hand, using protobuf (or flatbuffers, or cap'nproto, or JSON, or
msgpack, or whatever standard serialization format) has its share of
complexity. You have to find the end of the struct, you have to have a
parser both in kernel and in userspace, etc.

On the other hand, what we're seeing here is, to paraphrase Greenspun, an
ad hoc, informally-specified, bug-ridden, slow implementation of half of
protocol buffers ;-)

Given we're adding protobuf parser to the module, we can access it
relatively easily from any programming language, we do need need a special
syscall or include special headers (you need to get the ioctl number from
the running module version). And we're getting more features (i.e., ZigZag
encoding of integers, backwards compatibility) for free.

I can definitely see your point that sticking to C structs is simpler for
many things, but standard serialization could be simpler for others.

PS
Since you mentioned copy_from_user, I think it's worth mentioning that the
reason for not accessing user's memory directly, is that without
copy_from_user, the data could be swapped out in the middle of the memcpy.
Or at least this is one of the reasons.

Thanks,


On Mon, Mar 30, 2015 at 5:08 PM, Gilboa Davara <gilboad at gmail.com> wrote:

> On Mon, Mar 30, 2015 at 3:44 PM, Elazar Leibovich <elazarl at gmail.com>
> wrote:
> > Sounds good, thanks (although it'll be harder to use from non-C
> programs).
>
> I usually complement each kernel module with a user-mode C library
> wrapped inside a nice / easy to use (...) C++ class that handles the
> "difficult bits".
> It far easier to make a C++ class that handles idiot-proof (E.g.
> string commands) data than doing it inside the kernel.
> (Plus, when a user mode library goes up the flame due to bad input,
> you don't end up with a dead machine...)
>
> > Do you have a good idea how to stream information as a response to ioctl?
>
> Use TLV (type / length / value-array).
>
> typedef union _answer_data_ {
>        int __out error_code;
>        int   __out next_answer_time;
>        struct access_info {
>               int __out access_count;
>               int __out user_count;
>        };
> } answer_data_t;
>
> typedef struct _debug_request_
> {
>      request_t request_type;
>      void *request_pointer;
>      answer_t answer_type;  <--------------- Answer type. ( Enum, define,
> etc ).
>      int answer_buffer_size;  <--------------- User supplied answer
> buffer size (in answer_data union units).
>      int __out answer_buffer_used;  <-------- Answer buffer used size
> (number of answers copied by the kernel).
>      answer_data_t *answer_data_array; <-- Answer buffer array
> (user-mode pointer).
> } debug_request_t;
>
> Now, in-case of multiple answers, you simply
> copy_to_user((void *)&debug_request_copy->answer_data_array[count],
>                     (void *)&kernel_answer,
>                     sizeof(answer_data_t));
> count ++;
>
> .. To copy each answer to the user buffer.
> Once the array is depleted, the kernel function returns with a valid
> error code (E.g. -EEXIST) and tells to the user mode caller that
> there's additional data to be read (E.g. until the kernel returns
> -EWOULDBLOCK).
>
> Alternatively (and somewhat more complex) you can setup mmap over file
> handle, and simply map a static kernel buffer directly to the user
> mode. Though, unless you need to stream huge amounts of data and/or
> ioctl performance penalty [50-200ns] is too high, I wouldn't bother.
>
> BTW, Most likely your are aware of this, but just in case, never
> access the user mode buffer directly. When you first access the
> request buffer you copy it (copy_from_user) to kernel debug_request
> (no need to allocate it, considering its size, you shouldn't have any
> issues using stack, even on i386).
>
> - Gilboa
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.cs.huji.ac.il/pipermail/linux-il/attachments/20150330/747d552e/attachment.html>