netdev - Re: [PATCH 1/3] s390/vmlogrdr: Remove function pointer cast

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: <20240418102549.6056-B-hca@linux.ibm.com>
Date: Thu, 18 Apr 2024 12:25:49 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Heiko Carstens <hca@...ux.ibm.com>
Cc: Nathan Chancellor <nathan@...nel.org>, akpm@...ux-foundation.org,
        arnd@...db.de, gor@...ux.ibm.com, agordeev@...ux.ibm.com,
        borntraeger@...ux.ibm.com, svens@...ux.ibm.com, wintera@...ux.ibm.com,
        twinkler@...ux.ibm.com, linux-s390@...r.kernel.org,
        netdev@...r.kernel.org, llvm@...ts.linux.dev, patches@...ts.linux.dev
Subject: Re: [PATCH 1/3] s390/vmlogrdr: Remove function pointer cast

On Thu, Apr 18, 2024 at 11:54:38AM +0200, Heiko Carstens wrote:
> On Wed, Apr 17, 2024 at 11:24:35AM -0700, Nathan Chancellor wrote:
> > Clang warns (or errors with CONFIG_WERROR) after enabling
> > -Wcast-function-type-strict by default:
> > 
> >   drivers/s390/char/vmlogrdr.c:746:18: error: cast from 'void (*)(const void *)' to 'void (*)(struct device *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
> >     746 |                 dev->release = (void (*)(struct device *))kfree;
> >         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   1 error generated.
> > 
> > Add a standalone function to fix the warning properly, which addresses
> > the root of the warning that these casts are not safe for kCFI. The
> > comment is not really relevant after this change, so remove it.
> > 
> > Signed-off-by: Nathan Chancellor <nathan@...nel.org>
> > ---
> >  drivers/s390/char/vmlogrdr.c | 13 +++++--------
> >  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> > @@ -736,14 +740,7 @@ static int vmlogrdr_register_device(struct vmlogrdr_priv_t *priv)
> >  		dev->driver = &vmlogrdr_driver;
> >  		dev->groups = vmlogrdr_attr_groups;
> >  		dev_set_drvdata(dev, priv);
> > -		/*
> > -		 * The release function could be called after the
> > -		 * module has been unloaded. It's _only_ task is to
> > -		 * free the struct. Therefore, we specify kfree()
> > -		 * directly here. (Probably a little bit obfuscating
> > -		 * but legitime ...).
> > -		 */
> 
> Why is the comment not relevant after this change? Or better: why is it not
> valid before this change, which is why the code was introduced a very long
> time ago? Any reference?
> 
> I've seen the warning since quite some time, but didn't change the code
> before sure that this doesn't introduce the bug described in the comment.

>From only 20 years ago:

https://lore.kernel.org/all/20040316170812.GA14971@kroah.com/

The particular code (zfcp) was changed, so it doesn't have this code
(or never did?)  anymore, but for the rest this may or may not still
be valid.