sel_netif_init: 'err' is used uninitialized (was: Re: selinux: reduce the number of calls to synchronize_net() when flushing caches)

sel_netif_init: 'err' is used uninitialized (was: Re: selinux: reduce the number of calls to synchronize_net() when flushing caches)

[Geert Uytterhoeven]

On Wed, Aug 6, 2014 at 6:27 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Gitweb:     http://git.kernel.org/linus/;a=commit;h=615e51fdda6f274e94b1e905fcaf6111e0d9aa20
> Commit:     615e51fdda6f274e94b1e905fcaf6111e0d9aa20
> Parent:     f31e799459659ae88c341aeac16a8a5efb1271d4
> Refname:    refs/heads/master
> Author:     Paul Moore <pmoore@redhat.com>
> AuthorDate: Thu Jun 26 14:33:56 2014 -0400
> Committer:  Paul Moore <pmoore@redhat.com>
> CommitDate: Thu Jun 26 14:33:56 2014 -0400
>
>     selinux: reduce the number of calls to synchronize_net() when flushing caches
>
>     When flushing the AVC, such as during a policy load, the various
>     network caches are also flushed, with each making a call to
>     synchronize_net() which has shown to be expensive in some cases.
>     This patch consolidates the network cache flushes into a single AVC
>     callback which only calls synchronize_net() once for each AVC cache
>     flush.

> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c

> @@ -291,10 +282,6 @@ static __init int sel_netif_init(void)
>
>         register_netdevice_notifier(&sel_netif_netdev_notifier);
>
> -       err = avc_add_callback(sel_netif_avc_callback, AVC_CALLBACK_RESET);
> -       if (err)
> -               panic("avc_add_callback() failed, error %d\n", err);
> -
>         return err;

security/selinux/netif.c: In function ‘sel_netif_init’:
/scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
‘err’ is used uninitialized in this function

Should it just return 0, like before?
Or should it return the return value of register_netdevice_notifier() instead,
which also returns an error code? Or is that failure non-critical?

>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

sel_netif_init: 'err' is used uninitialized (was: Re: selinux: reduce the number of calls to synchronize_net() when flushing caches)

[Geert Uytterhoeven]

On Wed, Aug 6, 2014 at 6:27 PM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Gitweb:     http://git.kernel.org/linus/;a=commit;h=615e51fdda6f274e94b1e905fcaf6111e0d9aa20
> Commit:     615e51fdda6f274e94b1e905fcaf6111e0d9aa20
> Parent:     f31e799459659ae88c341aeac16a8a5efb1271d4
> Refname:    refs/heads/master
> Author:     Paul Moore <pmoore@redhat.com>
> AuthorDate: Thu Jun 26 14:33:56 2014 -0400
> Committer:  Paul Moore <pmoore@redhat.com>
> CommitDate: Thu Jun 26 14:33:56 2014 -0400
>
>     selinux: reduce the number of calls to synchronize_net() when flushing caches
>
>     When flushing the AVC, such as during a policy load, the various
>     network caches are also flushed, with each making a call to
>     synchronize_net() which has shown to be expensive in some cases.
>     This patch consolidates the network cache flushes into a single AVC
>     callback which only calls synchronize_net() once for each AVC cache
>     flush.

> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c

> @@ -291,10 +282,6 @@ static __init int sel_netif_init(void)
>
>         register_netdevice_notifier(&sel_netif_netdev_notifier);
>
> -       err = avc_add_callback(sel_netif_avc_callback, AVC_CALLBACK_RESET);
> -       if (err)
> -               panic("avc_add_callback() failed, error %d\n", err);
> -
>         return err;

security/selinux/netif.c: In function ‘sel_netif_init’:
/scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
‘err’ is used uninitialized in this function

Should it just return 0, like before?
Or should it return the return value of register_netdevice_notifier() instead,
which also returns an error code? Or is that failure non-critical?

>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: sel_netif_init: 'err' is used uninitialized (was: Re: selinux: reduce the number of calls to synchronize_net() when flushing caches)

[Paul Moore]

On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
> 
> security/selinux/netif.c: In function ‘sel_netif_init’:
> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
> ‘err’ is used uninitialized in this function
> 
> Should it just return 0, like before?
> Or should it return the return value of register_netdevice_notifier()
> instead, which also returns an error code? Or is that failure non-critical?

Hi,

I posted a fix for this two days ago to the SELinux list (see below).  As soon 
as -rc1 is released and linux-next is back in business I'll be pushing the 
patch to the SELinux #next branch.

 * http://marc.info/?l=selinux&m=140727033030054&w=2

-- 
paul moore
www.paul-moore.com

Re: sel_netif_init: 'err' is used uninitialized (was: Re: selinux: reduce the number of calls to synchronize_net() when flushing caches)

[Geert Uytterhoeven]

Hi Paul,

On Thu, Aug 7, 2014 at 4:26 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
>>
>> security/selinux/netif.c: In function ‘sel_netif_init’:
>> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
>> ‘err’ is used uninitialized in this function
>>
>> Should it just return 0, like before?
>> Or should it return the return value of register_netdevice_notifier()
>> instead, which also returns an error code? Or is that failure non-critical?
>
> Hi,
>
> I posted a fix for this two days ago to the SELinux list (see below).  As soon
> as -rc1 is released and linux-next is back in business I'll be pushing the
> patch to the SELinux #next branch.
>
>  * http://marc.info/?l=selinux&m=140727033030054&w=2

Thanks, I had a quick look for it, but couldn't find it.
It's always a good idea to CC lkml for such issues in mainline.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: sel_netif_init: 'err' is used uninitialized

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 1291 bytes --]

From: Paul Moore <paul@paul-moore.com>
Date: Thu, 07 Aug 2014 10:26:14 -0400

> On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
>> 
>> security/selinux/netif.c: In function ¡sel_netif_init¢:
>> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
>> ¡err¢ is used uninitialized in this function
>> 
>> Should it just return 0, like before?
>> Or should it return the return value of register_netdevice_notifier()
>> instead, which also returns an error code? Or is that failure non-critical?
> 
> Hi,
> 
> I posted a fix for this two days ago to the SELinux list (see below).  As soon 
> as -rc1 is released and linux-next is back in business I'll be pushing the 
> patch to the SELinux #next branch.
> 
>  * http://marc.info/?l=selinux&m=140727033030054&w=2

With respect to this and the lack-of-inline warning fix we spoke about
yesterday, why are you waiting and only pushing such bug fixes into your
"next" branch?

Those sort of things should be sent to Linus now to correct the errors
introduced during the merge window, as I have done last night for all
of the networking merge fallout.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: sel_netif_init: 'err' is used uninitialized

[David Miller]

From: Paul Moore <paul@paul-moore.com>
Date: Thu, 07 Aug 2014 10:26:14 -0400

> On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
>> 
>> security/selinux/netif.c: In function ‘sel_netif_init’:
>> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
>> ‘err’ is used uninitialized in this function
>> 
>> Should it just return 0, like before?
>> Or should it return the return value of register_netdevice_notifier()
>> instead, which also returns an error code? Or is that failure non-critical?
> 
> Hi,
> 
> I posted a fix for this two days ago to the SELinux list (see below).  As soon 
> as -rc1 is released and linux-next is back in business I'll be pushing the 
> patch to the SELinux #next branch.
> 
>  * http://marc.info/?l=selinux&m=140727033030054&w=2

With respect to this and the lack-of-inline warning fix we spoke about
yesterday, why are you waiting and only pushing such bug fixes into your
"next" branch?

Those sort of things should be sent to Linus now to correct the errors
introduced during the merge window, as I have done last night for all
of the networking merge fallout.

Re: sel_netif_init: 'err' is used uninitialized

[Paul Moore]

On Thursday, August 07, 2014 12:04:54 PM David Miller wrote:
> From: Paul Moore <paul@paul-moore.com>
> Date: Thu, 07 Aug 2014 10:26:14 -0400
> 
> > On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
> > 
> >> 
> >> security/selinux/netif.c: In function ‘sel_netif_init’:
> >> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
> >> ‘err’ is used uninitialized in this function
> >> 
> >> Should it just return 0, like before?
> >> Or should it return the return value of register_netdevice_notifier()
> >> instead, which also returns an error code? Or is that failure
> >> non-critical?
> > 
> > Hi,
> > 
> > I posted a fix for this two days ago to the SELinux list (see below).  As
> > soon as -rc1 is released and linux-next is back in business I'll be
> > pushing the patch to the SELinux #next branch.
> > 
> >  * http://marc.info/?l=selinux&m=140727033030054&w=2
> 
> With respect to this and the lack-of-inline warning fix we spoke about
> yesterday, why are you waiting and only pushing such bug fixes into your
> "next" branch?

Simply put, I didn't think the patches were significant enough to push at this 
point in time.
 
> Those sort of things should be sent to Linus now to correct the errors
> introduced during the merge window, as I have done last night for all
> of the networking merge fallout.

I'll (re)post the patches with a pull request in just a moment, CC'ing all the 
various mailing lists and you guys can figure out who best to merge them.

-- 
paul moore
www.paul-moore.com

Re: sel_netif_init: 'err' is used uninitialized

[Paul Moore]

On Thursday, August 07, 2014 12:04:54 PM David Miller wrote:
> From: Paul Moore <paul@paul-moore.com>
> Date: Thu, 07 Aug 2014 10:26:14 -0400
> 
> > On Thursday, August 07, 2014 12:31:15 PM Geert Uytterhoeven wrote:
> > 
> >> 
> >> security/selinux/netif.c: In function ‘sel_netif_init’:
> >> /scratch/geert/linux/linux-m68k/security/selinux/netif.c:285: warning:
> >> ‘err’ is used uninitialized in this function
> >> 
> >> Should it just return 0, like before?
> >> Or should it return the return value of register_netdevice_notifier()
> >> instead, which also returns an error code? Or is that failure
> >> non-critical?
> > 
> > Hi,
> > 
> > I posted a fix for this two days ago to the SELinux list (see below).  As
> > soon as -rc1 is released and linux-next is back in business I'll be
> > pushing the patch to the SELinux #next branch.
> > 
> >  * http://marc.info/?l=selinux&m=140727033030054&w=2
> 
> With respect to this and the lack-of-inline warning fix we spoke about
> yesterday, why are you waiting and only pushing such bug fixes into your
> "next" branch?

Simply put, I didn't think the patches were significant enough to push at this 
point in time.
 
> Those sort of things should be sent to Linus now to correct the errors
> introduced during the merge window, as I have done last night for all
> of the networking merge fallout.

I'll (re)post the patches with a pull request in just a moment, CC'ing all the 
various mailing lists and you guys can figure out who best to merge them.

-- 
paul moore
www.paul-moore.com

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html