linux-kernel - RE: [PATCH 1/1] linux/container_of.h: Warn about loss of constness

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4218173bd72b4f1899d4c41a8e251f0d@AcuMS.aculab.com>
Date:   Mon, 24 Oct 2022 21:24:18 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "'Rafael J. Wysocki'" <rafael@...nel.org>,
        Kees Cook <keescook@...omium.org>
CC:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/1] linux/container_of.h: Warn about loss of constness

From: Rafael J. Wysocki
> Sent: 24 October 2022 18:51
...
> > It looks like it was designed to handle the cases where the pointer was
> > ERR_OR_NULL:
> >
> >        IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
> >                ((type *)(__mptr - offsetof(type, member))); })
> >
> > i.e. just pass through the NULL/ERR instead of attempting the cast,
> > which would fail spectacularly. :)
> >
> > It seems like this version should actually be used everywhere instead of
> > nowhere... (i.e. just drop container_of() and rename container_of_safe()
> > to container_of())
> 
> As a rule, though, users of container_of() don't check the pointer
> returned by it against NULL, so I'm not sure how much of an
> improvement that would be.
> 
> If NULL is passed to container_of(), there will be a spectacular
> failure, sooner or later ...

Certainly there isn't much difference between dereferencing
a -Exxxx value and -Exxxx - offsetof().
Both are in the same page - hopefully not mapped?

Missing ERR/NULL checks are a problem but adding one inside
container_of() doesn't really help.

You might as well add an explicit test before using container_of()
rather than adding one inside it AND requiring a test afterwards.
I don't think the compiler can assume the subtraction doesn't
generate NULL - so must check twice.

I've not even sure how many of the functions that can check can
ever actually be passed an invalid pointer.
Normally the caller bails out and returns the error before
passing it on.
The kernel really doesn't check every function parameter for
validity - it has to assume the caller is doing something sensible.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)