[systemd-devel] [PATCH 2/3] udev: check return value of uname.

[Lennart Poettering]

On Wed, 27.03.13 16:58, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> 
> On Wed, Mar 27, 2013 at 04:50:48PM +0100, Lennart Poettering wrote:
> > On Wed, 27.03.13 15:16, Vaclav Pavlin (vpavlin at redhat.com) wrote:
> > 
> > > From: Václav Pavlín <vpavlin at redhat.com>
> > > 
> > > ---
> > >  src/udev/udevd.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/udev/udevd.c b/src/udev/udevd.c
> > > index b30bedf..379168d 100644
> > > --- a/src/udev/udevd.c
> > > +++ b/src/udev/udevd.c
> > > @@ -819,7 +819,9 @@ static void static_dev_create_from_modules(struct udev *udev)
> > >          char buf[4096];
> > >          FILE *f;
> > >  
> > > -        uname(&kernel);
> > > +        if (uname(&kernel) < 0)
> > > +                return;
> > > +
> > 
> > I wouldn't mind enclosing this in assert_se(), since this specific
> > system call cannot realistically fail.
> Hi,
> assert might not be nice, because of random selinux denials... Wouldn't it be
> better to continue if possible?

Not really. SELinux checks labels of objects against labels of processes
really. uname(), gettimeofday(), getpid() and things like that aren't
really affected, since they don't really do anything on any objects.

I mean, doing assert_se() around them is already more than these
syscalls deserve I guess, since realistically they cannot fail. And if
they do fail, then there's something really really wrong with the
security policy. Much much wronger than just a mislabelled file
somewhere... In such a corner case I think it is fine to asserts.

To me this is like checking for IO errors on stderr after logging
something, or if we'd verify each time when we invoke snprintf() that
the resuling string parses back to the same number with strtoull.

But anyway, it's a bit of a bikeshedding thing about where you draw the
line between "this is clearly a programming error condition" and "this
is clearly a runtime error condition"...

Lennart

-- 
Lennart Poettering - Red Hat, Inc.