| 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
| ||
|
Message-ID: <f6e24dfa-450e-4de6-87a6-6fcf7d83888d@embeddedor.com>
Date: Wed, 29 Jan 2025 18:35:18 +1030
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Greg KH <gregkh@...uxfoundation.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: Dan Carpenter <dan.carpenter@...aro.org>, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2][next] container_of: add container_first() macro
On 29/01/25 16:24, 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?
This is more about the cases where the member _must_ be first in the
structure. See below for an example related to -Wflex-array-member-not-at-end
>
>>
>> Co-developed-by: Dan Carpenter <dan.carpenter@...aro.org>
>> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>> ---
>>
>> I will be using this in my -Wflex-array-member-not-at-end patches. :)
>
> Confused, I'd like to see some users first please.
When addressing the -Wflex-array-member-not-at-end warnings, the common
scenario is when we have to separate the flexible-array member from the
rest of the members in the flexible structure, as shown below [1]:
struct bplus_header
{
struct_group_tagged(bplus_header_fixed, __hdr,
u8 flags; /* bit 0 - high bit of first free entry offset
bit 5 - we're pointed to by an fnode,
the data btree or some ea or the
main ea bootage pointer ea_secno
bit 6 - suggest binary search (unused)
bit 7 - 1 -> (internal) tree of anodes
0 -> (leaf) list of extents */
u8 fill[3];
u8 n_free_nodes; /* free nodes in following array */
u8 n_used_nodes; /* used nodes in following array */
__le16 first_free; /* offset from start of header to
first free node in array */
);
union {
/* (internal) 2-word entries giving subtree pointers */
DECLARE_FLEX_ARRAY(struct bplus_internal_node, internal);
/* (external) 3-word entries giving sector runs */
DECLARE_FLEX_ARRAY(struct bplus_leaf_node, external);
} u;
};
struct_group_tagged() creates a new type: `struct bplus_header_fixed`, we
then use the newly created type to change the type of the middle objects
causing the warnings, and with that the warnings are gone:
@@ -453,7 +455,7 @@ struct fnode
__le16 flags; /* bit 1 set -> ea_secno is an anode */
/* bit 8 set -> directory. first & only extent
points to dnode. */
- struct bplus_header btree; /* b+ tree, 8 extents or 12 subtrees */
+ struct bplus_header_fixed btree; /* b+ tree, 8 extents or 12 subtrees */
union {
struct bplus_leaf_node external[8];
struct bplus_internal_node internal[12];
@@ -495,7 +497,7 @@ struct anode
__le32 self; /* pointer to this anode */
__le32 up; /* parent anode or fnode */
- struct bplus_header btree; /* b+tree, 40 extents or 60 subtrees */
+ struct bplus_header_fixed btree; /* b+tree, 40 extents or 60 subtrees */
union {
struct bplus_leaf_node external[40];
struct bplus_internal_node internal[60];
However, this newly created type, or rather the member `__hdr` (also
created when calling struct_group_tagged()) _must_ always be the first
member in the flexible struct `struct bplus_header`.
Then we need to use container_first() to retrieve a pointer to the flexible
structure [2], via which we can access the flexible-array member when
necessary:
diff --git a/fs/hpfs/anode.c b/fs/hpfs/anode.c
index c14c9a035ee0c0..a366f6ac71e436 100644
--- a/fs/hpfs/anode.c
+++ b/fs/hpfs/anode.c
@@ -27,7 +27,7 @@ secno hpfs_bplus_lookup(struct super_block *s, struct inode *inode,
a = le32_to_cpu(btree->u.internal[i].down);
brelse(bh);
if (!(anode = hpfs_map_anode(s, a, &bh))) return -1;
- btree = &anode->btree;
+ btree = container_first(&anode->btree, struct bplus_header, __hdr);
goto go_down;
}
hpfs_error(s, "sector %08x not found in internal anode %08x", sec, a);
@@ -69,12 +69,16 @@ secno hpfs_add_sector_to_btree(struct super_block *s, secno node, int fnod, unsi
int n;
unsigned fs;
int c1, c2 = 0;
+ struct bplus_header *anode_btree = container_first(&anode->btree, struct bplus_header, __hdr);
+ struct bplus_header *ranode_btree = container_first(&ranode->btree, struct bplus_header, __hdr);
+ struct bplus_header *fnode_btree = container_first(&fnode->btree, struct bplus_header, __hdr);
+
So, if we use container_first() (or container_of_first(), you tell
which name you prefer), we are asserting that that `__hdr` member
is always the first member in `struct bplus_header`.
I've explained all this at Plumbers last year. In any case, let
me know if you need more clarification. :)
Thanks!
--
Gustavo
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/diff/fs/hpfs/hpfs.h?h=testing/wfamnae-next20250124&id=f66219294267a2fba220f4f3118e11c5cda63d0b
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/diff/fs/hpfs/anode.c?h=testing/wfamnae-next20250124&id=f66219294267a2fba220f4f3118e11c5cda63d0b
Powered by blists - more mailing lists