[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <05a36e03-a1d3-bd2e-1e95-86d71add6a8b@huaweicloud.com>
Date: Tue, 30 Dec 2025 10:52:02 +0800
From: Li Nan <linan666@...weicloud.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: arnd@...db.de, gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
"wanghai (M)" <wanghai38@...wei.com>
Subject: Re: [PATCH] char: lp: Fix NULL pointer dereference of cad
在 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.
--
Thanks,
Nan
Powered by blists - more mailing lists