[3.11.y.z extended stable] Patch "vfs: fix subtle use-after-free of pipe_inode_info" has been added to staging queue
Brad Figg
brad.figg at canonical.com
Thu Dec 12 15:56:04 UTC 2013
Kengyu,
I'm afraid Tim is correct and we've passed the cutoff for the 12.04.4
kernel.
There should be a new kernel with this commit shortly after 12.04.4
releases.
Brad
On 12/12/2013 07:04 AM, Tim Gardner wrote:
> Kengyu - I agree that this is an important patch for 3.11 stable.
> However, SRU testing for 12.04.4 is sufficiently far along that I would
> not want to interrupt (and restart) testing for a symptom that requires
> many hours to manifest. In other words, its not a boot essential fix. We
> could certainly have a kernel waiting in -proposed by the time 12.04.4
> is released.
>
> Brad - thoughts ?
>
> rtg
>
> On 12/12/2013 07:29 AM, Keng-Yu Lin wrote:
>> Hi there:
>> By any chance can this patch make itself included in the 12.04.4
>> point release?
>>
>> It is found that this patch fixes kernel oops occurring with the
>> current Ubuntu 3.11-based kernel. (the oops happens in roughly 30 hrs
>> in a stress test environment on a server machine).
>>
>> The patch is already in the upstream 3.10.y and 3.12.y stable trees,
>> but has no change to be in the last 3.11.y release (this is 3.11.10) .
>>
>> cheers,
>> -kengyu
>>
>> On Wed, Dec 11, 2013 at 10:41 PM, Luis Henriques
>> <luis.henriques at canonical.com> wrote:
>>> This is a note to let you know that I have just added a patch titled
>>>
>>> vfs: fix subtle use-after-free of pipe_inode_info
>>>
>>> to the linux-3.11.y-queue branch of the 3.11.y.z extended stable tree
>>> which can be found at:
>>>
>>> http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.11.y-queue
>>>
>>> If you, or anyone else, feels it should not be added to this tree, please
>>> reply to this email.
>>>
>>> For more information about the 3.11.y.z tree, see
>>> https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
>>>
>>> Thanks.
>>> -Luis
>>>
>>> ------
>>>
>>> From ddd487a2f0b5b804751a2972e99071d657d8a53d Mon Sep 17 00:00:00 2001
>>> From: Linus Torvalds <torvalds at linux-foundation.org>
>>> Date: Mon, 2 Dec 2013 09:44:51 -0800
>>> Subject: vfs: fix subtle use-after-free of pipe_inode_info
>>>
>>> commit b0d8d2292160bb63de1972361ebed100c64b5b37 upstream.
>>>
>>> The pipe code was trying (and failing) to be very careful about freeing
>>> the pipe info only after the last access, with a pattern like:
>>>
>>> spin_lock(&inode->i_lock);
>>> if (!--pipe->files) {
>>> inode->i_pipe = NULL;
>>> kill = 1;
>>> }
>>> spin_unlock(&inode->i_lock);
>>> __pipe_unlock(pipe);
>>> if (kill)
>>> free_pipe_info(pipe);
>>>
>>> where the final freeing is done last.
>>>
>>> HOWEVER. The above is actually broken, because while the freeing is
>>> done at the end, if we have two racing processes releasing the pipe
>>> inode info, the one that *doesn't* free it will decrement the ->files
>>> count, and unlock the inode i_lock, but then still use the
>>> "pipe_inode_info" afterwards when it does the "__pipe_unlock(pipe)".
>>>
>>> This is *very* hard to trigger in practice, since the race window is
>>> very small, and adding debug options seems to just hide it by slowing
>>> things down.
>>>
>>> Simon originally reported this way back in July as an Oops in
>>> kmem_cache_allocate due to a single bit corruption (due to the final
>>> "spin_unlock(pipe->mutex.wait_lock)" incrementing a field in a different
>>> allocation that had re-used the free'd pipe-info), it's taken this long
>>> to figure out.
>>>
>>> Since the 'pipe->files' accesses aren't even protected by the pipe lock
>>> (we very much use the inode lock for that), the simple solution is to
>>> just drop the pipe lock early. And since there were two users of this
>>> pattern, create a helper function for it.
>>>
>>> Introduced commit ba5bb147330a ("pipe: take allocation and freeing of
>>> pipe_inode_info out of ->i_mutex").
>>>
>>> Reported-by: Simon Kirby <sim at hostway.ca>
>>> Reported-by: Ian Applegate <ia at cloudflare.com>
>>> Acked-by: Al Viro <viro at zeniv.linux.org.uk>
>>> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
>>> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
>>> ---
>>> fs/pipe.c | 39 +++++++++++++++++++--------------------
>>> 1 file changed, 19 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/pipe.c b/fs/pipe.c
>>> index d2c45e1..0e0752e 100644
>>> --- a/fs/pipe.c
>>> +++ b/fs/pipe.c
>>> @@ -726,11 +726,25 @@ pipe_poll(struct file *filp, poll_table *wait)
>>> return mask;
>>> }
>>>
>>> +static void put_pipe_info(struct inode *inode, struct pipe_inode_info *pipe)
>>> +{
>>> + int kill = 0;
>>> +
>>> + spin_lock(&inode->i_lock);
>>> + if (!--pipe->files) {
>>> + inode->i_pipe = NULL;
>>> + kill = 1;
>>> + }
>>> + spin_unlock(&inode->i_lock);
>>> +
>>> + if (kill)
>>> + free_pipe_info(pipe);
>>> +}
>>> +
>>> static int
>>> pipe_release(struct inode *inode, struct file *file)
>>> {
>>> - struct pipe_inode_info *pipe = inode->i_pipe;
>>> - int kill = 0;
>>> + struct pipe_inode_info *pipe = file->private_data;
>>>
>>> __pipe_lock(pipe);
>>> if (file->f_mode & FMODE_READ)
>>> @@ -743,17 +757,9 @@ pipe_release(struct inode *inode, struct file *file)
>>> kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
>>> kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT);
>>> }
>>> - spin_lock(&inode->i_lock);
>>> - if (!--pipe->files) {
>>> - inode->i_pipe = NULL;
>>> - kill = 1;
>>> - }
>>> - spin_unlock(&inode->i_lock);
>>> __pipe_unlock(pipe);
>>>
>>> - if (kill)
>>> - free_pipe_info(pipe);
>>> -
>>> + put_pipe_info(inode, pipe);
>>> return 0;
>>> }
>>>
>>> @@ -1014,7 +1020,6 @@ static int fifo_open(struct inode *inode, struct file *filp)
>>> {
>>> struct pipe_inode_info *pipe;
>>> bool is_pipe = inode->i_sb->s_magic == PIPEFS_MAGIC;
>>> - int kill = 0;
>>> int ret;
>>>
>>> filp->f_version = 0;
>>> @@ -1130,15 +1135,9 @@ err_wr:
>>> goto err;
>>>
>>> err:
>>> - spin_lock(&inode->i_lock);
>>> - if (!--pipe->files) {
>>> - inode->i_pipe = NULL;
>>> - kill = 1;
>>> - }
>>> - spin_unlock(&inode->i_lock);
>>> __pipe_unlock(pipe);
>>> - if (kill)
>>> - free_pipe_info(pipe);
>>> +
>>> + put_pipe_info(inode, pipe);
>>> return ret;
>>> }
>>>
>>> --
>>> 1.8.3.2
>>>
>>>
>>> --
>>> kernel-team mailing list
>>> kernel-team at lists.ubuntu.com
>>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>>
>
>
--
Brad Figg brad.figg at canonical.com http://www.canonical.com
More information about the kernel-team
mailing list