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: <20240418192100.6741-A-hca@linux.ibm.com>
Date: Thu, 18 Apr 2024 21:21:00 +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 ...).
> > > > > > -		 */
> > 
> > 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.
> 
> I do not know the answer to that question (and I suspect there is
> nothing preventing ->release() from being called after module unload),
> so I'll just bring back the comment (although I'll need to adjust it
> since kfree() is not being used there directly anymore). Andrew, would
> you prefer a diff from what's in -mm or a v2?

I guess there is some confusion here :) My request was not to keep the
comment. I'm much rather afraid that the comment is still valid; and if
that is the case then your patch series adds three bugs, exactly what is
described in the comment.

Right now the release function is kfree which is always within the kernel
image, and therefore always a valid branch target. If however the code is
changed to what you propose, then the release function would be inside of
the module, which potentially does not exist anymore when the release
function is called, since the module was unloaded.
So the branch target would be invalid.