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]

Date:	Wed, 11 Mar 2009 23:39:03 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Eric Dumazet <dada1@...mosbay.com>
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

On Thu, 12 Mar 2009 07:10:38 +0100 Eric Dumazet <dada1@...mosbay.com> wrote:

> > 
> > 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...

"should"?

> > 
> > 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)

Well yes, a kludge like that seems a bit safer.

It's somewhat encouraging that we're apparently already doing fput()
from within keventd (although how frequently?).  There might be
problems with file locking, security code, etc from doing fput() from
an unexpected thread.  And then there are all the usual weird problem
with using the keventd queues which take a long time to get discovered.


> 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>.

The patches are large and scary and it would be a real problem to merge
them into 2.6.29 at this stage, let alone 2.6.25, etc.

Especially as the code which you sent out appears to be untested:

>  void fput(struct file *file)
>  {
> -	if (atomic_long_dec_and_test(&file->f_count))
> -		__fput(file);
> +	if (atomic_long_dec_and_test(&file->f_count)) {
> +		if (unlikely(!in_interrupt()))

                             ^

> +			fd_defer_queue(NULL, file);
> +		else
> +			__fput(file);
> +	}
>  }


--
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/