headers
J. R. Okajima | 27 May 2010 05:31
Picon
Favicon

Q. 'const'antify the members of a structure


Hello all,

The grsec/pax patches make member of struct brabra_operation 'const.'
I don't understand why they need these 'const'. They modifies some of
structures, but other structures.
What do they want to protect from what?

From my point of view, the keyword 'const' is essentially a feature of C
language and it never modifes the behaviour of software. It just
prohibits the assignment (or modification) to a variable which is
expected not to be modified.
In other word, it is a feature for programmers and doesn't enhance the
security level. Actually programmers can bypass 'const' easily by
indirect assignment.

Also the grsec/pax patches modifies some assignments to the member
of struct brabra_operation in mainline kernel, but they don't make the
confirmation fot that. For example, they replaced these assignments by
declaring a structure statically.
(I know they don't make member of file_operations 'const', so this is
exmaple is unrelated to the above.)

-	/* inherit and extend fuse_dev_operations */
-	cuse_channel_fops		= fuse_dev_operations;
-	cuse_channel_fops.owner		= THIS_MODULE;
-	cuse_channel_fops.open		= cuse_channel_open;
-	cuse_channel_fops.release	= cuse_channel_release;

+static const struct file_operations cuse_channel_fops = {
(Continue reading)

Permalink | Reply | 9 comments |
headers
pageexec | 27 May 2010 15:36
Picon
Favicon

Re: Q. 'const'antify the members of a structure

On 27 May 2010 at 12:31, J. R. Okajima wrote:

> The grsec/pax patches make member of struct brabra_operation 'const.'
> I don't understand why they need these 'const'. They modifies some of
> structures, but other structures.
> What do they want to protect from what?

the goal is to reduce the number of writable function pointers which is
part of a larger strategy for kernel self-protection.

> From my point of view, the keyword 'const' is essentially a feature of C
> language and it never modifes the behaviour of software. It just
> prohibits the assignment (or modification) to a variable which is
> expected not to be modified.

you're talking about the human point of view, but that's only part of the
whole picture, the machine point of view is quite different in fact. in
particular it's up to the toolchain and runtime environment to actually
enforce the const (read-only) property of code/data. as you may have inferred
it by now, PaX (and in particular KERNEXEC) does exactly that. even vanilla
kernels have been able to do this partially (DEBUG_RODATA) for a while now.

> In other word, it is a feature for programmers and doesn't enhance the
> security level. Actually programmers can bypass 'const' easily by
> indirect assignment.

try that under KERNEXEC (or even DEBUG_RODATA) and report back the results ;).

> Also the grsec/pax patches modifies some assignments to the member
> of struct brabra_operation in mainline kernel, but they don't make the
(Continue reading)

Permalink | Reply | 8 comments |
headers
J. R. Okajima | 27 May 2010 18:37
Picon
Favicon

Re: Q. 'const'antify the members of a structure


pageexec@...:
> you're talking about the human point of view, but that's only part of the
> whole picture, the machine point of view is quite different in fact. in
> particular it's up to the toolchain and runtime environment to actually
> enforce the const (read-only) property of code/data. as you may have inferred
> it by now, PaX (and in particular KERNEXEC) does exactly that. even vanilla
> kernels have been able to do this partially (DEBUG_RODATA) for a while now.

Do you mean that there is such environment which puts these const
members in a readonly section?
Although I agree that 'static const int *p = brabra' will be layouted in
readonly section, I am afraid 'struct {const int *p} foo' will not.
If you declare 'static const struct {const int *p} foo = {.p = brabra}',
then it will go to readonly. This is my main question.
At least in x86 32/64, 'p' in 'struct {const int *p} foo' can be
assigned indirectly.

> try that under KERNEXEC (or even DEBUG_RODATA) and report back the results ;).

After applying the grsec patch, DEBUG_RODATA is not selectable since it
depends upon BROKEN, is it?

> what makes you think that we don't do that? there are many things in PaX
> that assume certain 'invariants' in the kernel that have to be verified
> *manually* for each release. that's one reason why porting PaX is not a
> trivial business for example. so you can rest assured that we are very
> much aware of what dangers the 'forking' of such an ops structure presents
> and we check for changes each time we make a forward port.
(Continue reading)

Permalink | Reply | 7 comments |
headers
pageexec | 27 May 2010 21:43
Picon
Favicon

Re: Q. 'const'antify the members of a structure

On 28 May 2010 at 1:37, J. R. Okajima wrote:

> pageexec@...:
> > you're talking about the human point of view, but that's only part of the
> > whole picture, the machine point of view is quite different in fact. in
> > particular it's up to the toolchain and runtime environment to actually
> > enforce the const (read-only) property of code/data. as you may have inferred
> > it by now, PaX (and in particular KERNEXEC) does exactly that. even vanilla
> > kernels have been able to do this partially (DEBUG_RODATA) for a while now.
> 
> Do you mean that there is such environment which puts these const
> members in a readonly section?

well, it's a multi-step process, something i should have elaborated more in
the previous reply perhaps, so let's do it now.

as i said, the ultimate goal is to eliminate writable function pointers, or
at least reduce their numbers as much as possible (why that's a goal is
another question, the PaX docs should get you started). to reach that goal
first we have to identify the various function pointers in the kernel and
then see how we can make them const. these function pointers come in several
flavours, something like this:

  1. return addresses on the stack
  2. function addresses stored in global variables
  3. function addresses stored in dynamically allocated variables (on the heap)
  4. function addresses stored in local variables (on the stack)

of these we cannot do much about 1 and 4 (for this discussion, that is, there's
a separate line of development that addresses them). 2 and 3 can however be
(Continue reading)

Permalink | Reply | 6 comments |
headers
pageexec | 28 May 2010 10:15
Picon
Favicon

Re: Q. 'const'antify the members of a structure

On 28 May 2010 at 14:53, J. R. Okajima wrote:

> This is my case.
> I am developing a kernel module which is not merged into mainline (and
> it will not be). Some users use it with the grsec-pached kernel.
> In order to support various filesystems and their options, my module
> allocates struct foo_operations and sets members dynamically. And users
> got the compilation errors.

do you really need to allocate these structures at runtime? if all you need
is to modify existing ops structures then you can temporarily grant yourself
write access by using pax_open_kernel/pax_close_kernel. of course it's PaX
specific, it won't work with DEBUG_RODATA (you can abuse text_poke maybe ;).

> I don't understand why these 'const's enhances the security. But I
> think I can understand your explanation.

(writable) function pointers are the primary targets of memory corruption
based exploits, reducing their numbers reduces the attack surface.
Permalink | Reply | 4 comments |
headers
pageexec | 28 May 2010 11:52
Picon
Favicon

Re: Q. 'const'antify the members of a structure

On 28 May 2010 at 18:20, J. R. Okajima wrote:

> My fs cannot prepare all these variations statically since they are
> unknown,

i guess you could do that but it'd soon run into some sort of combinatorial explosion ;).

so in this case what you can do is similar to what PaX does in drivers/ata/libata-core.c:

-       ops->inherits = NULL;
+       ((struct ata_port_operations *)ops)->inherits = NULL;

this cast is a good way to document that you're doing something special (kernel
devs would say 'fishy' probably ;).
Permalink | Reply | 2 comments |
headers
pageexec | 28 May 2010 14:21
Picon
Favicon

Re: Q. 'const'antify the members of a structure

On 28 May 2010 at 20:45, J. R. Okajima wrote:

> 
> pageexec@...:
> > so in this case what you can do is similar to what PaX does in drivers/ata/libata-core.c:
> >
> > -       ops->inherits = NULL;
> > +       ((struct ata_port_operations *)ops)->inherits = NULL;
> 
> It is a 'const struct foo_ops *ops' case
> instead of 'struct foo_ops { void (* const f)() }' case, isn't it?

yes, it was just an example for how you use the explicit cast as a form
of documentation in such cases.
Permalink | Reply | 0 comments |
headers
J. R. Okajima | 28 May 2010 07:53
Picon
Favicon

Re: Q. 'const'antify the members of a structure


pageexec@...:
> the last remaining case is 3 and we have it on good authority (namely, Al Viro)
> that such constructs are not desirable in linux in general and should never
> occur in fact. now C doesn't make it possible to express this property easily,
	:::
> what this does is that at compile time it'll fail on code which tries to
> directly write into the some_op field of such a structure (it does allow
> initialization which is what you need for case 2), which in turn implies
> that such structures cannot be dynamically allocated since they'd need
> direct writes to set up the individual members after the allocation. so
> this is the real reason for making field members const, it implicitly forces
> the programmer to think twice about what he's doing. or rather, it would,
	:::

This is my case.
I am developing a kernel module which is not merged into mainline (and
it will not be). Some users use it with the grsec-pached kernel.
In order to support various filesystems and their options, my module
allocates struct foo_operations and sets members dynamically. And users
got the compilation errors.
I don't understand why these 'const's enhances the security. But I
think I can understand your explanation.

Thanx for explanation.

J. R. Okajima
Permalink | Reply | 5 comments |
headers
J. R. Okajima | 28 May 2010 11:20
Picon
Favicon

Re: Q. 'const'antify the members of a structure


pageexec@...:
> do you really need to allocate these structures at runtime? if all you need
> is to modify existing ops structures then you can temporarily grant yourself
> write access by using pax_open_kernel/pax_close_kernel. of course it's PaX
> specific, it won't work with DEBUG_RODATA (you can abuse text_poke maybe ;).

My module is a stackable filesystem which refers multiple other fs. For
example, when one of its member fs is ext2 with 'xip' option and the
other is no 'xip', then I should prepare two kinds of
address_space_operations, one has ->get_xip_mem() and the other doesn't.
As you might know, ext2 has several variations of
address_space_operrations (statically) and sets one of them to a file.

My fs cannot prepare all these variations statically since they are
unknown, so allocates address_space_operrations dynamically and sets the
member functions in it following the underlaying ext2's
address_space_operrations.

Finally,
- struct brabra operations in my fs should be set following the
  corresponding data in other fs.
- there is no gurantee when/which fs (and struct) comes to my fs.

J. R. Okajima
Permalink | Reply | 3 comments |
headers
J. R. Okajima | 28 May 2010 13:45
Picon
Favicon

Re: Q. 'const'antify the members of a structure


pageexec@...:
> so in this case what you can do is similar to what PaX does in drivers/ata/libata-core.c:
>
> -       ops->inherits = NULL;
> +       ((struct ata_port_operations *)ops)->inherits = NULL;

It is a 'const struct foo_ops *ops' case
instead of 'struct foo_ops { void (* const f)() }' case, isn't it?

J. R. Okajima
Permalink | Reply | 1 comment |

Gmane