linux-kernel - Re: [PATCH] fs: fput() can be called from interrupt context

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: <49B8A75E.6040409@cosmosbay.com>
Date:	Thu, 12 Mar 2009 07:10:38 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Jeff Moyer <jmoyer@...hat.com>, Avi Kivity <avi@...hat.com>,
	linux-aio <linux-aio@...ck.org>, zach.brown@...cle.com,
	bcrl@...ck.org, linux-kernel@...r.kernel.org,
	Davide Libenzi <davidel@...ilserver.org>,
	Christoph Lameter <cl@...ux-foundation.org>
Subject: Re: [PATCH] fs: fput() can be called from interrupt context

Andrew Morton a écrit :
> On Thu, 12 Mar 2009 06:18:26 +0100 Eric Dumazet <dada1@...mosbay.com> wrote:
> 
>> Eric Dumazet wrote :
>>> Path could be :
>>>
>>> 1) fput() changes so that calling it from interrupt context is possible
>>>    (Using a working queue to make sure __fput() is called from process context)
>>>
>>> 2) Changes aio to use fput() as is (and zap its internal work_queue and aio_fput_routine() stuff)
>>>
>>> 3) Once atomic_long_dec_and_test(&filp->f_count) only performed in fput(),
>>>    SLAB_DESTROY_BY_RCU for "struct file" get back :)
>>>
>> Please find first patch against linux-2.6
>>
>> Next patch (2) can cleanup aio code, but it probably can wait linux-2.6.30
>>
>> Thank you
>>
>> [PATCH] fs: fput() can be called from interrupt context
>>
>> Current aio/eventfd code can call fput() from interrupt context, which is
>> not allowed.
> 
> The changelog forgot to tell us where this happens, and under what
> circumstances.
> 
> See, there might be other ways of fixing the bug,

Sure

> 
>> In order to fix the problem and prepare SLAB_DESTROY_BY_RCU use for "struct file"
>> allocation/freeing in 2.6.30, we might extend existing workqueue infrastructure and
>> allow fput() to be called from interrupt context.
>>
>> This unfortunalty adds a pointer to 'struct file'.
>>
>> Signed-off-by: Eric Dumazet <dada1@...mosbay.com>
>> ---
>>  fs/file.c               |   55 ++++++++++++++++++++++++++------------
>>  fs/file_table.c         |   10 +++++-
>>  include/linux/fdtable.h |    1
>>  include/linux/fs.h      |    1
>>  4 files changed, 49 insertions(+), 18 deletions(-)
> 
> which might not have some or all of the above problems.
> 
> 
> I assume you're referring to really_put_req(), and commit
> 9c3060bedd84144653a2ad7bea32389f65598d40.
> 
>>>From the above email straggle I extract "If user program closes
> eventfd, then inflight AIO requests can trigger a bug" and I don't
> immediately see anything in there which would prevent this.
> 
> Did you reproduce the bug, and confirm that the patch fixes it?

take Davide program : http://www.xmailserver.org/eventfd-aio-test.c

and add at line 318 :
close(afd);

It should produce the kernel bug...
> 
> Are there simpler ways of fixing it?  Maybe sneak a call to
> wait_for_all_aios() into the right place?  I doubt if it's performance
> critical, as nobody seems to have ever hit the bug.

Take the time to check how fs/aio.c handle the fput(req->ki_filp) case
(or read my 2nd patch, it should spot the thing)

If you want to add another kludge to properly fput(req->ki_eventfd),
be my guest :-(

> 
> Bear in mind that if the bug _is_ real then it's now out there, and
> we would like a fix which is usable by 2.6.<two-years-worth>.
> 
> etcetera..




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/