linux-hardening - Re: [PATCH v2][next] container_of: add container

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: <2295987a-7794-4404-834e-9218349a6f45@stanley.mountain>
Date: Wed, 29 Jan 2025 10:29:24 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2][next] container_of: add container_first() macro

On Wed, Jan 29, 2025 at 06:54:48AM +0100, Greg KH wrote:
> On Wed, Jan 29, 2025 at 03:56:01PM +1030, Gustavo A. R. Silva wrote:
> > This is like container_of_const() but it contains an assert to
> > ensure that it's using the first member in the structure.
> 
> But why?  If you "know" it's the first member, just do a normal cast.
> If you don't, then you probably shouldn't be caring about this anyway,
> right?
> 

Heh.  I had a long coversation with someone where I tried to explain four
times that casting the first member was a thing.  In the end, they were
able to accept that it works but only "accidentally."  We merged their
patch as a cleanup.

https://lore.kernel.org/all/20250116193432.716db3a2@gandalf.local.home/

We have quite a bit of code like:

drivers/iommu/iommufd/iommufd_private.h
   243  static inline struct iommufd_ioas *iommufd_get_ioas(struct iommufd_ctx *ictx,
   244                                                      u32 id)
   245  {
   246          return container_of(iommufd_get_object(ictx, id,
   247                                                 IOMMUFD_OBJ_IOAS),
   248                              struct iommufd_ioas, obj);
   249  }

It's just a cast like you say, but it looks like pointer math.  It would
be more readable as container_of_first().

The weird thing is that when people check if (IS_ERR()) on a
container_of() then normally the code is correct, but when they check for
NULL then normally the NULL check is a harmless no-op.

regards,
dan carpenter