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

Heiko Carstens
2024-04-18T10:27:36Z

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@kernel.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.