On 2/13/24 1:37 AM, Johannes Berg wrote:
On Mon, 2024-02-12 at 15:22 -0800, Ben Greear wrote:
I'm seeing a crash due to accessing an 'sta' object in this method that is '-2' as integer.
It fails the initial check for null STA, but I'm thinking it might should check for IS_ERR(sta)
as well.
(I have my own patch that references sta before the IS_ERR check later in the code, and this
causes the crash I'm seeing. I guess upstream will not crash in this situation.).
Indeed.
My question: Is the patch below a preferred approach, or should I add special checks to where I
access sta and only exit the method lower where it already has the IS_ERR(sta) check?
You can do whatever you want in your tree, but I guess generally I'd
advocate you assume that the code does what it should ;-)
In this case, ERR_PTR(-ENOENT) is used to indicate the station is being
deleted, but has not yet been fully removed, and so indeed we still want
to reclaim the frames here correctly, which the code does.
The comment below even kind of explains that?
If sta is NULL, we should still reclaim the frames? If so the check earlier in the code where
it returns early if sta is NULL could be deleted, and add a null check down near the IS_ERR
check?
I can (and have) fixed the bug in my code, I'm trying to understand if the driver
itself needs improving here to cover an edge case.
Thanks,
Ben
johannes
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
