Object-oriented design patterns in the kernel, part 1 [LWN.net]

Object-oriented design patterns in the kernel, part 1

Posted Jun 2, 2011 0:26 UTC (Thu) by elanthis (guest, #6227) [Link]

I don't have a link, but it's a real problem that's been observed in the wild.

It's not a compiler bug by any means. It's correct, normal, expected behavior. There's no reason to initialize padding space in most circumstances, and doing so is just a loss of efficiency for no gain for those overwhelmingly common cases.

The only cases where this ever comes up as a problem are in security contexts (e.g. kernel stack data leakage) and when you're trying to do "undefined" operations like hashing a chunk of memory representing a struct. In the former case, you should be more careful and explicitly zero out memory via memset or calloc (or equivalent APIs) which you need to do anything for just about every other data structure you use so it's not a big deal, and in the later case you should be beaten with a stick until you stop doing such stupid things. :)

Object-oriented design patterns in the kernel, part 1

Posted Jun 2, 2011 0:39 UTC (Thu) by foom (subscriber, #14868) [Link]

I ran into it when writing in-memory structs to disk (to be mmap'd in later). It was a problem that there was random garbage in between the data, since then it was not possible to md5sum two files and tell if they were the same.

Furthermore, gcc emits *terrible* code when explicitly initializing a bunch of integer members to 0 (especially where some of them are bitfields). My solution was to make a parent class call bzero to clear out sizeof(*this), and not explicitly initialize any of the initially-zero integer members. Made faster code *and* didn't have any padding issues. (But now someone's probably going to tell me that doing that is undefined and that gcc 4.8's optimizer is going to decide to transform my app into nethack, because I invoked some undefined behavior).

Object-oriented design patterns in the kernel, part 1

Posted Jun 2, 2011 13:48 UTC (Thu) by cesarb (subscriber, #6266) [Link]

I believe that, unless your class has only plain old data members (the kind you would find in a C struct), doing a bzero of *this has the potential of blowing up. The compiler can add extra hidden members (like the vtable pointer), and you would be overwriting them.

That is, I believe what you are doing is safe only if:

- All the integer and bitfield members (and only them) are in the parent class;
- The parent class is zeroing only itself (that is, it is doing the sizeof(*this) itself, instead of being passed that value by the child class);
- The parent class has no virtual member functions or anything else that could make the compiler add C++ magic to it.

That said, I am no language lawyer, and would not be surprised if several clauses scattered all over the standard combine to say that even then you are still doing something undefined.

Why not move all these variables into a plain C-style struct, add it to your object as a member, and zero it on the constructor? That way sounds much safer to me, and I doubt the C++ standard would break it (since breaking it would break compatibility with C).

Object-oriented design patterns in the kernel, part 1

Posted Jun 3, 2011 11:20 UTC (Fri) by liljencrantz (subscriber, #28458) [Link]

What makes you assume the parent is talking C++ and not C? Am I missing something or are you?

Object-oriented design patterns in the kernel, part 1

Posted Jun 3, 2011 13:51 UTC (Fri) by sethml (subscriber, #8471) [Link]

Presumably because *this means something special in C++, and is not a common thing to write in C.

I've written this bug in C++ (memset(this, 0, sizeof(*this))) - clobbering your vtable pointer is pretty annoying to debug. My horrible hacky fix was just to zero the data portion of the class: memset(&firstMember, 0, (char *)(&lastMember + 1) - (char *)&firstMember)

Object-oriented design patterns in the kernel, part 1

Posted Jun 4, 2011 10:11 UTC (Sat) by liljencrantz (subscriber, #28458) [Link]

I always use the name «this» for the object pointer when doing OOP in C. From what I've seen, this is a common convention.

Object-oriented design patterns in the kernel, part 1

Posted Jun 2, 2011 17:49 UTC (Thu) by marcH (subscriber, #57642) [Link]

> It's not a compiler bug by any means. It's correct, normal, expected behavior. There's no reason to initialize padding space in most circumstances, and doing so is just a loss of efficiency for no gain for those overwhelmingly common cases.

Performance over security: a very important design choice of C, one to keep in mind at all times. One carried over to C++. One responsible for zillions of security flaws written by thousands of programmers who should rather have used another, safer language (or stayed in bed).