willing to involve in KSPP

willing to involve in KSPP

[Shyam Saini]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

 Hello Everyone,

I'm Shyam, currently working in linux kernel and embedded domain.

I'm interested work in Kernel Self Protection Project.

Would you please suggest which task should i pick ?

I'd like to pick some simple task to get started.

I see a list of todo [1] but this page is not updated since 31st October

I don't have much experience in linux kernel so I'm not sure to pick which
particular task,  please suggest me  some task.

Looking forward to hearing from you.

[1] https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Work


Thanks a lot,
Shyam

[-- Attachment #2: Type: text/html, Size: 1142 bytes --]

Re: willing to involve in KSPP

[Kees Cook]

On Wed, Apr 10, 2019 at 6:34 AM Shyam Saini <mayhs11saini@gmail.com> wrote:
>  Hello Everyone,

Hi!

> I'm Shyam, currently working in linux kernel and embedded domain.
>
> I'm interested work in Kernel Self Protection Project.
>
> Would you please suggest which task should i pick ?
>
> I'd like to pick some simple task to get started.
> I see a list of todo [1] but this page is not updated since 31st October

I've updated this list now. Sorry for the delay!

> I don't have much experience in linux kernel so I'm not sure to pick which particular task,  please suggest me  some task.

One item on there that looks pretty simple and would get you started
would be to regularized the use of three different macros that all do
the same thing: replace sizeof_field() and SIZEOF_FIELD() with the
more common FIELD_SIZEOF()

$ git grep '\bsizeof_field\b' | wc -l
30
$ git grep '\bSIZEOF_FIELD\b' | wc -l
2
$ git grep '\bFIELD_SIZEOF\b' | wc -l
291

Which likely means moving the FIELD_SIZEOF macro into
include/linux/stddef.h for reuse in offsetofend().

> Looking forward to hearing from you.
>
> [1] https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Work
>
>
> Thanks a lot,

Thanks for reaching out!

-- 
Kees Cook

Re: willing to involve in KSPP

[Shyam Saini]

Hi Kees,

> > I'm Shyam, currently working in linux kernel and embedded domain.
> >
> > I'm interested work in Kernel Self Protection Project.
> >
> > Would you please suggest which task should i pick ?
> >
> > I'd like to pick some simple task to get started.
> > I see a list of todo [1] but this page is not updated since 31st October
>
> I've updated this list now. Sorry for the delay!

Thanks for this update.

> > I don't have much experience in linux kernel so I'm not sure to pick which particular task,  please suggest me  some task.
>
> One item on there that looks pretty simple and would get you started
> would be to regularized the use of three different macros that all do
> the same thing: replace sizeof_field() and SIZEOF_FIELD() with the
> more common FIELD_SIZEOF()
>
> $ git grep '\bsizeof_field\b' | wc -l
> 30

These are spreaded all over kernel source tree.

> $ git grep '\bSIZEOF_FIELD\b' | wc -l
> 2

This was added recently and I think we will need a checkpatch entry to
warn developers about this.

> $ git grep '\bFIELD_SIZEOF\b' | wc -l
> 291
>
> Which likely means moving the FIELD_SIZEOF macro into
> include/linux/stddef.h for reuse in offsetofend().

My only concern is, if it picked by different maintainers then it may
break some other trees.
I think it should be picked by single maintainer so which tree should
I submit these changes ?


Thanks a lot,
Shyam

Re: willing to involve in KSPP

[Kees Cook]

On Sat, Apr 13, 2019 at 5:23 AM Shyam Saini <mayhs11saini@gmail.com> wrote:
>
> Hi Kees,
>
> > > I'm Shyam, currently working in linux kernel and embedded domain.
> > >
> > > I'm interested work in Kernel Self Protection Project.
> > >
> > > Would you please suggest which task should i pick ?
> > >
> > > I'd like to pick some simple task to get started.
> > > I see a list of todo [1] but this page is not updated since 31st October
> >
> > I've updated this list now. Sorry for the delay!
>
> Thanks for this update.
>
> > > I don't have much experience in linux kernel so I'm not sure to pick which particular task,  please suggest me  some task.
> >
> > One item on there that looks pretty simple and would get you started
> > would be to regularized the use of three different macros that all do
> > the same thing: replace sizeof_field() and SIZEOF_FIELD() with the
> > more common FIELD_SIZEOF()
> >
> > $ git grep '\bsizeof_field\b' | wc -l
> > 30
>
> These are spreaded all over kernel source tree.
>
> > $ git grep '\bSIZEOF_FIELD\b' | wc -l
> > 2
>
> This was added recently and I think we will need a checkpatch entry to
> warn developers about this.
>
> > $ git grep '\bFIELD_SIZEOF\b' | wc -l
> > 291
> >
> > Which likely means moving the FIELD_SIZEOF macro into
> > include/linux/stddef.h for reuse in offsetofend().
>
> My only concern is, if it picked by different maintainers then it may
> break some other trees.
> I think it should be picked by single maintainer so which tree should
> I submit these changes ?

I would just make this a pair of patches: one that does the treewide
changes in all files, and then one that removes the unused macros. I
would CC Andrew Morton on the patch; he would likely be willing to
take it.

-- 
Kees Cook

Re: willing to involve in KSPP

[Shyam Saini]

Hi Kees,


> > Hi Kees,
> >
> > > > I'm Shyam, currently working in linux kernel and embedded domain.
> > > >
> > > > I'm interested work in Kernel Self Protection Project.
> > > >
> > > > Would you please suggest which task should i pick ?
> > > >
> > > > I'd like to pick some simple task to get started.
> > > > I see a list of todo [1] but this page is not updated since 31st October
> > >
> > > I've updated this list now. Sorry for the delay!
> >
> > Thanks for this update.
> >
> > > > I don't have much experience in linux kernel so I'm not sure to pick which particular task,  please suggest me  some task.
> > >
> > > One item on there that looks pretty simple and would get you started
> > > would be to regularized the use of three different macros that all do
> > > the same thing: replace sizeof_field() and SIZEOF_FIELD() with the
> > > more common FIELD_SIZEOF()
> > >
> > > $ git grep '\bsizeof_field\b' | wc -l
> > > 30
> >
> > These are spreaded all over kernel source tree.
> >
> > > $ git grep '\bSIZEOF_FIELD\b' | wc -l
> > > 2
> >
> > This was added recently and I think we will need a checkpatch entry to
> > warn developers about this.
> >
> > > $ git grep '\bFIELD_SIZEOF\b' | wc -l
> > > 291
> > >
> > > Which likely means moving the FIELD_SIZEOF macro into
> > > include/linux/stddef.h for reuse in offsetofend().
> >
> > My only concern is, if it picked by different maintainers then it may
> > break some other trees.
> > I think it should be picked by single maintainer so which tree should
> > I submit these changes ?
>
> I would just make this a pair of patches: one that does the treewide
> changes in all files, and then one that removes the unused macros. I
> would CC Andrew Morton on the patch; he would likely be willing to
> take it.

Thanks a lot for your suggestion.

As you may already know that I've submitted the patch as per your
suggestion but I'm still not
sure how this FIELD macro uniformity in kernel is related to security.
Is there any future plans
based on this cleanup ?  Please excuse my lack of knowledge on the subject.

Re: willing to involve in KSPP

[Kees Cook]

On Mon, Apr 15, 2019 at 10:44 PM Shyam Saini <mayhs11saini@gmail.com> wrote:
> As you may already know that I've submitted the patch as per your
> suggestion but I'm still not

Thanks, i saw that! I'm hoping akpm will take it.

> sure how this FIELD macro uniformity in kernel is related to security.
> Is there any future plans
> based on this cleanup ?  Please excuse my lack of knowledge on the subject.

It was noticed while doing the HARDENED_USERCOPY work, and was a small
task that I wrote down at the time. It's more "related" to security
than actually making a security change.

-- 
Kees Cook

Re: willing to involve in KSPP

[Shyam Saini]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

> On Mon, Apr 15, 2019 at 10:44 PM Shyam Saini <mayhs11saini@gmail.com>
wrote:
> > As you may already know that I've submitted the patch as per your
> > suggestion but I'm still not
>
> Thanks, i saw that! I'm hoping akpm will take it.

Would you please acknowledge these patches ?


> > sure how this FIELD macro uniformity in kernel is related to security.
> > Is there any future plans
> > based on this cleanup ?  Please excuse my lack of knowledge on the
subject.
>
> It was noticed while doing the HARDENED_USERCOPY work, and was a small
> task that I wrote down at the time. It's more "related" to security
> than actually making a security change.

Thanks for this info.

Could you please suggest me series of more tasks so that I don't have to
bother you
each and every time for new task. :-)


Thanks a lot,
Shyam

[-- Attachment #2: Type: text/html, Size: 1425 bytes --]

Re: willing to involve in KSPP

[Kees Cook]

On Wed, Apr 17, 2019 at 9:22 AM Shyam Saini <mayhs11saini@gmail.com> wrote:
>
>
> > On Mon, Apr 15, 2019 at 10:44 PM Shyam Saini <mayhs11saini@gmail.com> wrote:
> > > As you may already know that I've submitted the patch as per your
> > > suggestion but I'm still not
> >
> > Thanks, i saw that! I'm hoping akpm will take it.
>
> Would you please acknowledge these patches ?

Other folks had questions about them -- I was assuming you would
answer those questions, etc. It sounded like people wanted to see the
series reduced to a single patch to do everything at once, so a v2
might be a good idea.

> Could you please suggest me series of more tasks so that I don't have to bother you
> each and every time for new task. :-)

Sure! Here are two:

- WARN on kfree() of ERR_PTR range
(or ignore them, as if it were NULL)

Right now kfree() only checks for NULL and zero sized allocations.
It'd be nice if it would WARN() (and skip) freeing the ERR_PTR range
(the -1 through -4095 addresses). The clever thing grsecurity did was
redefine the ZERO_SIZE allocation pointer to be -4096, and update the
ZERO_OR_NULL_PTR check[1]:

-#define ZERO_SIZE_PTR ((void *)16)
+#define ZERO_SIZE_PTR              \
+({                     \
+   BUILD_BUG_ON(!(MAX_ERRNO & ~PAGE_MASK));\
+   (void *)(-MAX_ERRNO-1L);        \
+})

-#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
-               (unsigned long)ZERO_SIZE_PTR)
+#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) - 1 >= (unsigned
long)ZERO_SIZE_PTR - 1)

So while the change is small (and be sure to give them credit[2]), it
might take some convincing various memory allocation folks that this
change makes sense. (I like it because it moves the ZERO_SIZE_PTR out
of userspace -- technically it was still in the NULL page, but better
to collect it up in unmapped memory at the top of the address space.)
This change should make a bunch of bugs around mishandling of ERR_PTRs
stand out better. People I would Cc:

Matthew Wilcox <willy@infradead.org>
Christoph Lameter <cl@linux.com>


Another one that needs some checking is:

- audit and fix all misuse of NLA_STRING

This is a following up on noticing the misuse of NLA_STRING (no NUL
terminator), getting used with regular string functions (that expect a
NUL termination):
https://lore.kernel.org/lkml/1519329289.2637.12.camel@sipsolutions.net/T/#u

It'd be nice if someone could inspect all the NLA_STRING
representations and find if there are any other problems like this
(and see if there was a good way to systemically fix the problem).


-Kees

[1] https://github.com/linux-scraping/linux-grsecurity/blame/grsec-test/include/linux/slab.h#L127
[2] https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Get_Involved


--
Kees Cook

Re: willing to involve in KSPP

[Shyam Saini]

>
> On Wed, Apr 17, 2019 at 9:22 AM Shyam Saini <mayhs11saini@gmail.com> wrote:
> >
> >
> > > On Mon, Apr 15, 2019 at 10:44 PM Shyam Saini <mayhs11saini@gmail.com> wrote:
> > > > As you may already know that I've submitted the patch as per your
> > > > suggestion but I'm still not
> > >
> > > Thanks, i saw that! I'm hoping akpm will take it.
> >
> > Would you please acknowledge these patches ?
>
> Other folks had questions about them -- I was assuming you would

yes, I'll respond to the thread. Sorry for the delay.

> answer those questions, etc. It sounded like people wanted to see the
> series reduced to a single patch to do everything at once, so a v2
> might be a good idea.

There is one suggestion to  retain sizeof_field as alias of FIELD_SIZEOF
for the code compatibility reason.

Seems like it created confusion because of two separate patches.
Anyway, I'll post one single patch

> > Could you please suggest me series of more tasks so that I don't have to bother you
> > each and every time for new task. :-)
>
> Sure! Here are two:
> - WARN on kfree() of ERR_PTR range
> (or ignore them, as if it were NULL)
>
> Right now kfree() only checks for NULL and zero sized allocations.
> It'd be nice if it would WARN() (and skip) freeing the ERR_PTR range
> (the -1 through -4095 addresses). The clever thing grsecurity did was
> redefine the ZERO_SIZE allocation pointer to be -4096, and update the
> ZERO_OR_NULL_PTR check[1]:
>
> -#define ZERO_SIZE_PTR ((void *)16)
> +#define ZERO_SIZE_PTR              \
> +({                     \
> +   BUILD_BUG_ON(!(MAX_ERRNO & ~PAGE_MASK));\
> +   (void *)(-MAX_ERRNO-1L);        \
> +})
>
> -#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
> -               (unsigned long)ZERO_SIZE_PTR)
> +#define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) - 1 >= (unsigned
> long)ZERO_SIZE_PTR - 1)
>
> So while the change is small (and be sure to give them credit[2]), it
> might take some convincing various memory allocation folks that this
> change makes sense. (I like it because it moves the ZERO_SIZE_PTR out
> of userspace -- technically it was still in the NULL page, but better
> to collect it up in unmapped memory at the top of the address space.)
> This change should make a bunch of bugs around mishandling of ERR_PTRs
> stand out better. People I would Cc:
>
> Matthew Wilcox <willy@infradead.org>
> Christoph Lameter <cl@linux.com>
>
>
> Another one that needs some checking is:
>
> - audit and fix all misuse of NLA_STRING
>
> This is a following up on noticing the misuse of NLA_STRING (no NUL
> terminator), getting used with regular string functions (that expect a
> NUL termination):
> https://lore.kernel.org/lkml/1519329289.2637.12.camel@sipsolutions.net/T/#u
>
> It'd be nice if someone could inspect all the NLA_STRING
> representations and find if there are any other problems like this
> (and see if there was a good way to systemically fix the problem).
>
>
> -Kees
>
> [1] https://github.com/linux-scraping/linux-grsecurity/blame/grsec-test/include/linux/slab.h#L127
> [2] https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Get_Involved

Thanks for sharing these details. I'll proceed with this once
FIELD_SIZEOF clean up is done.