linux-kernel - Re: [PATCH] gfs2: Use container_of() for gfs2

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YnoQmuPgK9c5V8ZC@infradead.org>
Date:   Tue, 10 May 2022 00:13:30 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Bob Peterson <rpeterso@...hat.com>,
        kernel test robot <lkp@...el.com>,
        Andreas Gruenbacher <agruenba@...hat.com>,
        Bill Wendling <morbo@...gle.com>, cluster-devel@...hat.com,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Tom Rix <trix@...hat.com>,
        Steven Whitehouse <swhiteho@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gfs2: Use container_of() for gfs2_glock(aspace)

> +/* gfs2_glock_get(), "glock" must be first. */
> +struct glock_aspace {
> +	struct gfs2_glock	glock;
> +	struct address_space	mapping;
> +};

Why does glock need to be first?  The whole point of using container_of
is that we don't have to make that assumption.

>  static inline struct address_space *gfs2_glock2aspace(struct gfs2_glock *gl)
>  {
>  	if (gl->gl_ops->go_flags & GLOF_ASPACE)
> -		return (struct address_space *)(gl + 1);
> +		return &(container_of(gl, struct glock_aspace, glock)->mapping);

Do we even need the braces here?

>  	struct inode *inode = mapping->host;
>  	if (mapping->a_ops == &gfs2_meta_aops)
> -		return (((struct gfs2_glock *)mapping) - 1)->gl_name.ln_sbd;
> +		return container_of(mapping, struct glock_aspace, mapping)->glock.gl_name.ln_sbd;

A local variable would be really nice for the reader here to decompose
this a bit:

		struct glock_aspace *a =
			container_of(mapping, struct glock_aspace, mapping);

		return a->glock.gl_name.ln_sbd;