Re: Qwery regarding Selinux Change Id context

2017-12-01 Thread Aman Sharma 
Hi All,

Thanks for the information.

But after resetting the semanage User/login, and moving the targeted folder
to old one and then install the default target. then also its still showing
the
Id context as context=*system_u:system_r:unconfined_t:s0-s0:c0.c1023.*

*What I observed is after changing the permission using semanage command
also, its still showing the system_u:system_r. *

*Check the semanage login/User output :*

*semanage login -l*

*Login Name   SELinux User MLS/MCS RangeService*

*__default__  unconfined_u s0-s0:c0.c1023   **
*root unconfined_u s0-s0:c0.c1023   **
*system_u system_u s0-s0:c0.c1023   **


*semanage user -l*

*Labeling   MLS/   MLS/  *
*SELinux UserPrefix MCS Level  MCS Range
SELinux Roles*

*guest_u user   s0 s0
 guest_r*
*rootuser   s0 s0-s0:c0.c1023
 staff_r sysadm_r system_r unconfined_r*
*staff_u user   s0 s0-s0:c0.c1023
 staff_r sysadm_r system_r unconfined_r*
*sysadm_uuser   s0 s0-s0:c0.c1023
 sysadm_r*
*system_uuser   s0 s0-s0:c0.c1023
 system_r unconfined_r*
*unconfined_uuser   s0 s0-s0:c0.c1023
 system_r unconfined_r*
*user_u  user   s0 s0
 user_r*
*xguest_uuser   s0 s0
 xguest_r*


Looks like its related to some other issue. What you think about this.

Thanks
Aman


On Sat, Dec 2, 2017 at 1:05 AM, Simon Sekidde  wrote:

>
>
> - Original Message -
> > From: "Stephen Smalley" 
> > To: "Simon Sekidde" , "Aman Sharma" <
> amansh.shar...@gmail.com>
> > Cc: "SELinux" 
> > Sent: Friday, December 1, 2017 2:28:17 PM
> > Subject: Re: Qwery regarding Selinux Change Id context
> >
> > On Fri, 2017-12-01 at 14:16 -0500, Simon Sekidde wrote:
> > >
> > > - Original Message -
> > > > From: "Aman Sharma" 
> > > > To: "SELinux" 
> > > > Sent: Thursday, November 30, 2017 11:26:21 PM
> > > > Subject: Re: Fwd: Qwery regarding Selinux Change Id context
> > > >
> > > > Hi ,
> > > >
> > > > mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> > > >
> > > > This targeted folder is not there.
> > > >
> > > > After searching I got the below result :
> > > >
> > > > find / -type d -name "*targeted" -print
> > > >
> > > > /usr/share/selinux/targeted
> > > > /etc/selinux/targeted
> > > >
> > > > Pleas let me know your comments.
> > > >
> > >
> > > Run
> > >
> > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > yum reinstall selinux-policy-targeted
> >
> > He already tried that and it allegedly didn't help.  It also seems to
> > leave you without a /etc/selinux/targeted/active/seusers file for some
> > reason, such that semanage login -l shows nothing.  But you can recover
> > by copying /etc/selinux/targeted/seusers to
> > /etc/selinux/targeted/active/seusers.  That's a bug.
> >
>
> Interesting. Thanks for spotting this.
>
> > >
> > > Also what does this output show
> > >
> > > ps -aelfZ | grep -i ssh
> > >
> > > >
> > > > On Fri, Dec 1, 2017 at 1:49 AM, Dominick Grift  > > > com>
> > > > wrote:
> > > >
> > > > > On Thu, Nov 30, 2017 at 11:10:43AM +0530, Aman Sharma wrote:
> > > > > > Hi Stephen,
> > > > > >
> > > > > > After reseting Selinux targeted folder also (the steps you
> > > > > > mentioned in
> > > > >
> > > > > the
> > > > > > earlier mail), Still its showing the same Id context i.e.
> > > > > >
> > > > > > *id*
> > > > > > *uid=0(root) gid=0(root) groups=0(root)
> > > > > > context=system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > > > > > *[root@cucm2 ~]# id -Z*
> > > > > > *system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > > > > >
> > > > > > *And semanage login -l is showing blank output. *
> > > > > >
> > > > > > *Do you have any idea about this.*
> > > > > >
> > > > > > *Thanks*
> > > > > > *Aman*
> > > > >
> > > > > Try the same procedure again but this time also do before
> > > > > reinstalling:
> > > > >
> > > > > mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Nov 29, 2017 at 11:04 PM, Stephen Smalley  > > > > > a.gov>
> > > > >
> > > > > wrote:
> > > > > >
> > > > > > > On Wed, 2017-11-29 at 22:01 +0530, Aman Sharma wrote:
> > > > > > > > After resetting boolean also, showing the same id context.
> > > > > > >
> > > > > > > And did you try fully resetting your policy as I suggested:
> > > > > > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > > > > > yum reinstall selinux-policy-targeted
> > > > > > > reboot
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, Nov 29, 2017 at 9:50 PM, Stephen Smalley  > > > > > > > .nsa.gov>
> > > > > > > > wrote:
> > > > > > >

Re: [PATCH v2] selinux: ensure the context is NUL terminated in security_context_to_sid_core()

2017-12-01 Thread William Roberts 
On Fri, Dec 1, 2017 at 1:31 PM, Paul Moore  wrote:
> From: Paul Moore 
>
> The syzbot/syzkaller automated tests found a problem in
> security_context_to_sid_core() during early boot (before we load the
> SELinux policy) where we could potentially feed context strings without
> NUL terminators into the strcmp() function.
>
> We already guard against this during normal operation (after the SELinux
> policy has been loaded) by making a copy of the context strings and
> explicitly adding a NUL terminator to the end.  The patch extends this
> protection to the early boot case (no loaded policy) by moving the context
> copy earlier in security_context_to_sid_core().
>
> Reported-by: syzbot 
> Signed-off-by: Paul Moore 
> ---
>  security/selinux/ss/services.c |   18 --
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 33cfe5d3d6cb..d05496deb229 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1413,27 +1413,25 @@ static int security_context_to_sid_core(const char 
> *scontext, u32 scontext_len,
> if (!scontext_len)
> return -EINVAL;
>
> +   /* Copy the string to allow changes and ensure a NUL terminator */
> +   scontext2 = kmemdup_nul(scontext, scontext_len, gfp_flags);
> +   if (!scontext2)
> +   return -ENOMEM;
> +
> if (!ss_initialized) {
> int i;
>
> for (i = 1; i < SECINITSID_NUM; i++) {
> -   if (!strcmp(initial_sid_to_string[i], scontext)) {
> +   if (!strcmp(initial_sid_to_string[i], scontext2)) {
> *sid = i;
> -   return 0;
> +   goto out;
> }
> }
> *sid = SECINITSID_KERNEL;
> -   return 0;
> +   goto out;
> }
> *sid = SECSID_NULL;
>
> -   /* Copy the string so that we can modify the copy as we parse it. */
> -   scontext2 = kmalloc(scontext_len + 1, gfp_flags);
> -   if (!scontext2)
> -   return -ENOMEM;
> -   memcpy(scontext2, scontext, scontext_len);
> -   scontext2[scontext_len] = 0;
> -
> if (force) {
> /* Save another copy for storing in uninterpreted form */
> rc = -ENOMEM;
>
>

I like negative diffstats.

Re: [PATCH] selinux: ensure the context is NULL terminated in security_context_to_sid_core()

2017-12-01 Thread Paul Moore 
On Fri, Dec 1, 2017 at 2:20 PM, Stephen Smalley  wrote:
> On Fri, 2017-12-01 at 10:34 -0500, Paul Moore wrote:
>> On Thu, Nov 30, 2017 at 6:44 PM, William Roberts
>>  wrote:
>> > On Thu, Nov 30, 2017 at 8:52 AM, Paul Moore 
>> > wrote:
>> > > From: Paul Moore 
>> > >
>> > > The syzbot/syzkaller automated tests found a problem in
>> > > security_context_to_sid_core() during early boot (before we load
>> > > the
>> > > SELinux policy) where we could potentially feed context strings
>> > > without
>> > > NULL terminators into the strcmp() function.
>> > >
>> > > We already guard against this during normal operation (after the
>> > > SELinux
>> > > policy has been loaded) by making a copy of the context strings
>> > > and
>> > > explicitly adding a NULL terminator to the end.  The patch
>> > > extends this
>> > > protection to the early boot case (no loaded policy) by moving
>> > > the context
>> > > copy earlier in security_context_to_sid_core().
>> > >
>> > > Reported-by: syzbot 
>> > > Signed-off-by: Paul Moore 
>> > > ---
>> > >  security/selinux/ss/services.c |   20 ++--
>> > >  1 file changed, 10 insertions(+), 10 deletions(-)
>> > >
>> > > diff --git a/security/selinux/ss/services.c
>> > > b/security/selinux/ss/services.c
>> > > index 33cfe5d3d6cb..6ec4cdc8af8f 100644
>> > > --- a/security/selinux/ss/services.c
>> > > +++ b/security/selinux/ss/services.c
>> > > @@ -1413,27 +1413,27 @@ static int
>> > > security_context_to_sid_core(const char *scontext, u32
>> > > scontext_len,
>> > > if (!scontext_len)
>> > > return -EINVAL;
>> > >
>> > > +   /* Copy the string to allow changes and ensure a NULL
>> > > terminator */
>> > > +   scontext2 = kmalloc(scontext_len + 1, gfp_flags);
>> > > +   if (!scontext2)
>> > > +   return -ENOMEM;
>> > > +   memcpy(scontext2, scontext, scontext_len);
>> > > +   scontext2[scontext_len] = 0;
>> >
>> > Call me crazy, but can't we use kmemdup_nul() here?
>>
>> Crazy good idea ;)
>>
>> I didn't realize that function existed, I'll respin.  Thanks.
>
> Also note that it should be NUL not NULL in the patch subject and
> description.  '\0' vs (void*)0

Yes, thanks.  Muscle memory has me just typing "NULL" everywhere,
regardless of the context.

-- 
paul moore
security @ redhat

Re: Qwery regarding Selinux Change Id context

2017-12-01 Thread Stephen Smalley 
On Fri, 2017-12-01 at 14:16 -0500, Simon Sekidde wrote:
> 
> - Original Message -
> > From: "Aman Sharma" 
> > To: "SELinux" 
> > Sent: Thursday, November 30, 2017 11:26:21 PM
> > Subject: Re: Fwd: Qwery regarding Selinux Change Id context
> > 
> > Hi ,
> > 
> > mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> > 
> > This targeted folder is not there.
> > 
> > After searching I got the below result :
> > 
> > find / -type d -name "*targeted" -print
> > 
> > /usr/share/selinux/targeted
> > /etc/selinux/targeted
> > 
> > Pleas let me know your comments.
> > 
> 
> Run
> 
> mv /etc/selinux/targeted /etc/selinux/targeted.old 
> yum reinstall selinux-policy-targeted

He already tried that and it allegedly didn't help.  It also seems to
leave you without a /etc/selinux/targeted/active/seusers file for some
reason, such that semanage login -l shows nothing.  But you can recover
by copying /etc/selinux/targeted/seusers to
/etc/selinux/targeted/active/seusers.  That's a bug.

> 
> Also what does this output show 
> 
> ps -aelfZ | grep -i ssh 
> 
> > 
> > On Fri, Dec 1, 2017 at 1:49 AM, Dominick Grift  > com>
> > wrote:
> > 
> > > On Thu, Nov 30, 2017 at 11:10:43AM +0530, Aman Sharma wrote:
> > > > Hi Stephen,
> > > > 
> > > > After reseting Selinux targeted folder also (the steps you
> > > > mentioned in
> > > 
> > > the
> > > > earlier mail), Still its showing the same Id context i.e.
> > > > 
> > > > *id*
> > > > *uid=0(root) gid=0(root) groups=0(root)
> > > > context=system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > > > *[root@cucm2 ~]# id -Z*
> > > > *system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > > > 
> > > > *And semanage login -l is showing blank output. *
> > > > 
> > > > *Do you have any idea about this.*
> > > > 
> > > > *Thanks*
> > > > *Aman*
> > > 
> > > Try the same procedure again but this time also do before
> > > reinstalling:
> > > 
> > > mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> > > 
> > > > 
> > > > 
> > > > On Wed, Nov 29, 2017 at 11:04 PM, Stephen Smalley  > > > a.gov>
> > > 
> > > wrote:
> > > > 
> > > > > On Wed, 2017-11-29 at 22:01 +0530, Aman Sharma wrote:
> > > > > > After resetting boolean also, showing the same id context.
> > > > > 
> > > > > And did you try fully resetting your policy as I suggested:
> > > > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > > > yum reinstall selinux-policy-targeted
> > > > > reboot
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, Nov 29, 2017 at 9:50 PM, Stephen Smalley  > > > > > .nsa.gov>
> > > > > > wrote:
> > > > > > > On Wed, 2017-11-29 at 21:39 +0530, Aman Sharma wrote:
> > > > > > > > Hi Stephen,
> > > > > > > > 
> > > > > > > > After enabling the unconfined module and after reboot
> > > > > > > > also, Still
> > > > > > > > showing the same id context.
> > > > > > > > 
> > > > > > > > Is there any way to make the id context to normal state
> > > > > > > > again ?
> > > > > > > 
> > > > > > > Hmmm...try resetting all booleans too?  semanage boolean
> > > > > > > -D
> > > > > > > 
> > > > > > > Or you could be drastic and completely reset your policy:
> > > > > > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > > > > > yum reinstall selinux-policy-targeted
> > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > --
> > > > > > 
> > > > > > Thanks
> > > > > > Aman
> > > > > > Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> > > > 
> > > > 
> > > > 
> > > > --
> > > > 
> > > > Thanks
> > > > Aman
> > > > Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> > > 
> > > --
> > > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B
> > > 6B02
> > > https://sks-keyservers.net/pks/lookup?op=get=0x3B6C5F1D2C7
> > > B6B02
> > > Dominick Grift
> > > 
> > 
> > 
> > 
> > --
> > 
> > Thanks
> > Aman
> > Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> > 
> 
>

Re: [PATCH] selinux: ensure the context is NULL terminated in security_context_to_sid_core()

2017-12-01 Thread Stephen Smalley 
On Fri, 2017-12-01 at 10:34 -0500, Paul Moore wrote:
> On Thu, Nov 30, 2017 at 6:44 PM, William Roberts
>  wrote:
> > On Thu, Nov 30, 2017 at 8:52 AM, Paul Moore 
> > wrote:
> > > From: Paul Moore 
> > > 
> > > The syzbot/syzkaller automated tests found a problem in
> > > security_context_to_sid_core() during early boot (before we load
> > > the
> > > SELinux policy) where we could potentially feed context strings
> > > without
> > > NULL terminators into the strcmp() function.
> > > 
> > > We already guard against this during normal operation (after the
> > > SELinux
> > > policy has been loaded) by making a copy of the context strings
> > > and
> > > explicitly adding a NULL terminator to the end.  The patch
> > > extends this
> > > protection to the early boot case (no loaded policy) by moving
> > > the context
> > > copy earlier in security_context_to_sid_core().
> > > 
> > > Reported-by: syzbot 
> > > Signed-off-by: Paul Moore 
> > > ---
> > >  security/selinux/ss/services.c |   20 ++--
> > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/security/selinux/ss/services.c
> > > b/security/selinux/ss/services.c
> > > index 33cfe5d3d6cb..6ec4cdc8af8f 100644
> > > --- a/security/selinux/ss/services.c
> > > +++ b/security/selinux/ss/services.c
> > > @@ -1413,27 +1413,27 @@ static int
> > > security_context_to_sid_core(const char *scontext, u32
> > > scontext_len,
> > > if (!scontext_len)
> > > return -EINVAL;
> > > 
> > > +   /* Copy the string to allow changes and ensure a NULL
> > > terminator */
> > > +   scontext2 = kmalloc(scontext_len + 1, gfp_flags);
> > > +   if (!scontext2)
> > > +   return -ENOMEM;
> > > +   memcpy(scontext2, scontext, scontext_len);
> > > +   scontext2[scontext_len] = 0;
> > 
> > Call me crazy, but can't we use kmemdup_nul() here?
> 
> Crazy good idea ;)
> 
> I didn't realize that function existed, I'll respin.  Thanks.

Also note that it should be NUL not NULL in the patch subject and
description.  '\0' vs (void*)0

> 
> > /**
> >  * kmemdup_nul - Create a NUL-terminated string from unterminated
> > data
> >  * @s: The data to stringify
> >  * @len: The size of the data
> >  * @gfp: the GFP mask used in the kmalloc() call when allocating
> > memory
> >  */
> > char *kmemdup_nul(const char *s, size_t len, gfp_t gfp)
> 
>

Re: Qwery regarding Selinux Change Id context

2017-12-01 Thread Simon Sekidde 


- Original Message -
> From: "Aman Sharma" 
> To: "SELinux" 
> Sent: Thursday, November 30, 2017 11:26:21 PM
> Subject: Re: Fwd: Qwery regarding Selinux Change Id context
> 
> Hi ,
> 
> mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> 
> This targeted folder is not there.
> 
> After searching I got the below result :
> 
> find / -type d -name "*targeted" -print
> 
> /usr/share/selinux/targeted
> /etc/selinux/targeted
> 
> Pleas let me know your comments.
> 

Run

mv /etc/selinux/targeted /etc/selinux/targeted.old 
yum reinstall selinux-policy-targeted

Also what does this output show 

ps -aelfZ | grep -i ssh 

> 
> On Fri, Dec 1, 2017 at 1:49 AM, Dominick Grift 
> wrote:
> 
> > On Thu, Nov 30, 2017 at 11:10:43AM +0530, Aman Sharma wrote:
> > > Hi Stephen,
> > >
> > > After reseting Selinux targeted folder also (the steps you mentioned in
> > the
> > > earlier mail), Still its showing the same Id context i.e.
> > >
> > > *id*
> > > *uid=0(root) gid=0(root) groups=0(root)
> > > context=system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > > *[root@cucm2 ~]# id -Z*
> > > *system_u:system_r:unconfined_t:s0-s0:c0.c1023*
> > >
> > > *And semanage login -l is showing blank output. *
> > >
> > > *Do you have any idea about this.*
> > >
> > > *Thanks*
> > > *Aman*
> >
> > Try the same procedure again but this time also do before reinstalling:
> >
> > mv /var/lib/selinux/targeted /var/lib/selinux/targeted.old
> >
> > >
> > >
> > > On Wed, Nov 29, 2017 at 11:04 PM, Stephen Smalley 
> > wrote:
> > >
> > > > On Wed, 2017-11-29 at 22:01 +0530, Aman Sharma wrote:
> > > > > After resetting boolean also, showing the same id context.
> > > >
> > > > And did you try fully resetting your policy as I suggested:
> > > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > > yum reinstall selinux-policy-targeted
> > > > reboot
> > > >
> > > > >
> > > > >
> > > > > On Wed, Nov 29, 2017 at 9:50 PM, Stephen Smalley 
> > > > > wrote:
> > > > > > On Wed, 2017-11-29 at 21:39 +0530, Aman Sharma wrote:
> > > > > > > Hi Stephen,
> > > > > > >
> > > > > > > After enabling the unconfined module and after reboot also, Still
> > > > > > > showing the same id context.
> > > > > > >
> > > > > > > Is there any way to make the id context to normal state again ?
> > > > > >
> > > > > > Hmmm...try resetting all booleans too?  semanage boolean -D
> > > > > >
> > > > > > Or you could be drastic and completely reset your policy:
> > > > > > mv /etc/selinux/targeted /etc/selinux/targeted.old
> > > > > > yum reinstall selinux-policy-targeted
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > Thanks
> > > > > Aman
> > > > > Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> > > >
> > >
> > >
> > >
> > > --
> > >
> > > Thanks
> > > Aman
> > > Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> >
> > --
> > Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
> > https://sks-keyservers.net/pks/lookup?op=get=0x3B6C5F1D2C7B6B02
> > Dominick Grift
> >
> 
> 
> 
> --
> 
> Thanks
> Aman
> Cell: +91 9990296404 |  Email ID : amansh.shar...@gmail.com
> 

-- 
Simon Sekidde
gpg: 5848 958E 73BA 04D3 7C06 F096 1BA1 2DBF 94BC 377E

Re: [PATCH] selinux: ensure the context is NULL terminated in security_context_to_sid_core()

2017-12-01 Thread Paul Moore 
On Thu, Nov 30, 2017 at 6:44 PM, William Roberts
 wrote:
> On Thu, Nov 30, 2017 at 8:52 AM, Paul Moore  wrote:
>> From: Paul Moore 
>>
>> The syzbot/syzkaller automated tests found a problem in
>> security_context_to_sid_core() during early boot (before we load the
>> SELinux policy) where we could potentially feed context strings without
>> NULL terminators into the strcmp() function.
>>
>> We already guard against this during normal operation (after the SELinux
>> policy has been loaded) by making a copy of the context strings and
>> explicitly adding a NULL terminator to the end.  The patch extends this
>> protection to the early boot case (no loaded policy) by moving the context
>> copy earlier in security_context_to_sid_core().
>>
>> Reported-by: syzbot 
>> Signed-off-by: Paul Moore 
>> ---
>>  security/selinux/ss/services.c |   20 ++--
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>> index 33cfe5d3d6cb..6ec4cdc8af8f 100644
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -1413,27 +1413,27 @@ static int security_context_to_sid_core(const char 
>> *scontext, u32 scontext_len,
>> if (!scontext_len)
>> return -EINVAL;
>>
>> +   /* Copy the string to allow changes and ensure a NULL terminator */
>> +   scontext2 = kmalloc(scontext_len + 1, gfp_flags);
>> +   if (!scontext2)
>> +   return -ENOMEM;
>> +   memcpy(scontext2, scontext, scontext_len);
>> +   scontext2[scontext_len] = 0;
>
> Call me crazy, but can't we use kmemdup_nul() here?

Crazy good idea ;)

I didn't realize that function existed, I'll respin.  Thanks.

> /**
>  * kmemdup_nul - Create a NUL-terminated string from unterminated data
>  * @s: The data to stringify
>  * @len: The size of the data
>  * @gfp: the GFP mask used in the kmalloc() call when allocating memory
>  */
> char *kmemdup_nul(const char *s, size_t len, gfp_t gfp)

-- 
paul moore
security @ redhat

Re: [BUG] kernel stack corruption during/after Netlabel error

2017-12-01 Thread David Ahern 
On 11/30/17 10:57 AM, Eric Dumazet wrote:
> I wonder if this should not be in a separate patch ?
> 
> Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
> TCP_SKB_CB layout to reduce cache line misses")  in linux 3.18
> 
> While VRF was added later.
> 
> If you agree, I will prepare a patch series, with different Fixes tag
> so that David can decide which path needs to be backported into each
> stable version.
> 

That's sound fine to me.

Re: [BUG] kernel stack corruption during/after Netlabel error

2017-12-01 Thread Eric Dumazet 
On Thu, 2017-11-30 at 10:30 -0700, David Ahern wrote:
> On 11/30/17 8:44 AM, David Ahern wrote:
> > On 11/30/17 3:50 AM, Eric Dumazet wrote:
> > > @@ -1631,24 +1659,6 @@ int tcp_v4_rcv(struct sk_buff *skb)
> > >  
> > >   th = (const struct tcphdr *)skb->data;
> > >   iph = ip_hdr(skb);
> > > - /* This is tricky : We move IPCB at its correct location
> > > into TCP_SKB_CB()
> > > -  * barrier() makes sure compiler wont play
> > > fool^Waliasing games.
> > > -  */
> > > - memmove(_SKB_CB(skb)->header.h4, IPCB(skb),
> > > - sizeof(struct inet_skb_parm));
> > > - barrier();
> > > -
> > > - TCP_SKB_CB(skb)->seq = ntohl(th->seq);
> > > - TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th-
> > > >syn + th->fin +
> > > - skb->len - th->doff * 4);
> > > - TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
> > > - TCP_SKB_CB(skb)->tcp_flags = tcp_flag_byte(th);
> > > - TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> > > - TCP_SKB_CB(skb)->ip_dsfield = ipv4_get_dsfield(iph);
> > > - TCP_SKB_CB(skb)->sacked  = 0;
> > > - TCP_SKB_CB(skb)->has_rxtstamp =
> > > - skb->tstamp || skb_hwtstamps(skb)-
> > > >hwtstamp;
> > > -
> > >  lookup:
> > >   sk = __inet_lookup_skb(_hashinfo, skb,
> > > __tcp_hdrlen(th), th->source,
> > >      th->dest, sdif, );
> > 
> > I believe moving the above is going to affect lookups with VRF. Let
> > me
> > take a look before this gets committed.
> > 
> 
> Eric:
> 
> Can you add this to the patch? Fixes socket lookups with VRF which
> stashes a flag in the cb.
> 
> Thanks,
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4e09398009c1..6c020015d556 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -849,7 +849,7 @@ static inline bool inet_exact_dif_match(struct
> net
> *net, struct sk_buff *skb)
>  {
>  #if IS_ENABLED(CONFIG_NET_L3_MASTER_DEV)
> if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
> -   skb && ipv4_l3mdev_skb(TCP_SKB_CB(skb)->header.h4.flags))
> +   skb && ipv4_l3mdev_skb(IPCB(skb)->flags))
> return true;
>  #endif
> return false;


I wonder if this should not be in a separate patch ?

Bug was added in 971f10eca186cab238c49daa91f703c5a001b0b1 ("tcp: better
TCP_SKB_CB layout to reduce cache line misses")  in linux 3.18

While VRF was added later.

If you agree, I will prepare a patch series, with different Fixes tag
so that David can decide which path needs to be backported into each
stable version.

Thanks.