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: <20240418151501.6056-C-hca@linux.ibm.com>
Date: Thu, 18 Apr 2024 17:15:01 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Nathan Chancellor <nathan@...nel.org>
Cc: 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

Hi Nathan,

> > > > -		/*
> > > > -		 * 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.
> 
> I guess relevant may not have been the correct word. Maybe obvious? I
> can keep the comment but I do not really see what it adds, although
> reading the above thread, I suppose it was added as justification for
> calling kfree() as ->release() for a 'struct device'? Kind of seems like
> that ship has sailed since I see this all over the place as a
> ->release() function. I do not see how this patch could have a function
> change beyond that but I may be misreading or misinterpreting your full
> comment.

That doesn't answer my question what prevents the release function
from being called after the module has been unloaded.

At least back then when the code was added it was a real bug.