[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fa47e75a-5296-7560-1127-4b6736ece97a@huaweicloud.com>
Date: Tue, 20 Jan 2026 15:59:50 +0800
From: Li Nan <linan666@...weicloud.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Al Viro <viro@...iv.linux.org.uk>, arnd@...db.de,
linux-kernel@...r.kernel.org, "wanghai (M)" <wanghai38@...wei.com>,
"yi.zhang@...wei.com" <yi.zhang@...wei.com>,
"yangerkun@...wei.com" <yangerkun@...wei.com>
Subject: Re: [PATCH] char: lp: Fix NULL pointer dereference of cad
在 2026/1/16 22:38, Greg KH 写道:
> On Tue, Dec 30, 2025 at 10:52:02AM +0800, Li Nan wrote:
>>
>>
>> 在 2025/12/30 10:10, Al Viro 写道:
>>> On Tue, Dec 30, 2025 at 09:51:43AM +0800, Li Nan wrote:
>>>> Friendly ping...
>>>
>>>>> @@ -569,10 +579,13 @@ static int lp_release(struct inode *inode, struct file *file)
>>>>> {
>>>>> unsigned int minor = iminor(inode);
>>>>> + if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
>>>>> + return -EINTR;
>>>
>>> ->release() return value is never checked, simply because there is nothing
>>> to do with it. It will *not* leave file opened - it will simply leak,
>>> with no way to recover from that.
>>>
>>> If you need to report some errors on close, do that in ->flush().
>>> If you ever see ->release() returning a non-zero value, you are very
>>> likely looking at deeply confused code.
>>>
>>> Don't do that. ->release() can't fail, period. It should've been
>>> void (*release)(struct file *), but for historical reasons it returns
>>> int and there are too many instances to change that.
>>
>> Thank you for your patient explanation.
>>
>> Would it be acceptable to switch to mutex_lock() here? Looking at the code
>> and historical changes, I don't see a compelling reason for the interruptible
>> function here.
>
> release should not stall forever like that, so be careful.
>
> Also, do you really have this hardware to test this with? I'm sure
Thank you for your reply.
Yes, I have the actual hardware for testing. I originally reproduced the
issue in QEMU and will run more comprehensive tests on the real device.
> there are loads of other cleanups / fixes needed in this driver that no
> one has done just because they don't have this hardware anymore.
> Getting a new maintainer for it would be great.
>
I agree the driver needs further cleanups and fixes. I'm happy to help with
testing and maintenance, and I'm willing to take on the maintainer role for
this driver going forward
> thanks,
>
> greg k-h
--
Thanks,
Nan
Powered by blists - more mailing lists