[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACT4Y+Y4e2gLbJAnNtaid3R_j_vM_e_1e6YuW0gRWPG2zA+w3Q@mail.gmail.com>
Date: Fri, 5 Feb 2016 14:33:08 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Hugh Dickins <hughd@...gle.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
syzkaller <syzkaller@...glegroups.com>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Sasha Levin <sasha.levin@...cle.com>,
Eric Dumazet <edumazet@...gle.com>
Subject: Re: fs: use-after-free in link_path_walk
On Sat, Jan 23, 2016 at 12:08 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Fri, Jan 22, 2016 at 11:33:09PM +0100, Dmitry Vyukov wrote:
>> Hello,
>>
>> The following program triggers a use-after-free in link_path_walk:
>> https://gist.githubusercontent.com/dvyukov/fc0da4b914d607ba8129/raw/b761243c44106d74f2173745132c82d179cbdc58/gistfile1.txt
>
> Hmm... Actually, I wonder if that had been triggerable since May. What
> happens is that unlike struct inode itself, shmem info->symlink is
> freed immediately, without an RCU delay. Easy to fix, fortunately...
>
> Could you check if the patch below fixes that for you?
Yes, it fixes the crash for me.
Thanks
> diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
> index a43f41c..4d4780c 100644
> --- a/include/linux/shmem_fs.h
> +++ b/include/linux/shmem_fs.h
> @@ -15,10 +15,7 @@ struct shmem_inode_info {
> unsigned int seals; /* shmem seals */
> unsigned long flags;
> unsigned long alloced; /* data pages alloced to file */
> - union {
> - unsigned long swapped; /* subtotal assigned to swap */
> - char *symlink; /* unswappable short symlink */
> - };
> + unsigned long swapped; /* subtotal assigned to swap */
> struct shared_policy policy; /* NUMA memory alloc policy */
> struct list_head swaplist; /* chain of maybes on swap */
> struct simple_xattrs xattrs; /* list of xattrs */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 38c5e72..440e2a7 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -701,8 +701,7 @@ static void shmem_evict_inode(struct inode *inode)
> list_del_init(&info->swaplist);
> mutex_unlock(&shmem_swaplist_mutex);
> }
> - } else
> - kfree(info->symlink);
> + }
>
> simple_xattrs_free(&info->xattrs);
> WARN_ON(inode->i_blocks);
> @@ -2549,13 +2548,12 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
> info = SHMEM_I(inode);
> inode->i_size = len-1;
> if (len <= SHORT_SYMLINK_LEN) {
> - info->symlink = kmemdup(symname, len, GFP_KERNEL);
> - if (!info->symlink) {
> + inode->i_link = kmemdup(symname, len, GFP_KERNEL);
> + if (!inode->i_link) {
> iput(inode);
> return -ENOMEM;
> }
> inode->i_op = &shmem_short_symlink_operations;
> - inode->i_link = info->symlink;
> } else {
> inode_nohighmem(inode);
> error = shmem_getpage(inode, 0, &page, SGP_WRITE, NULL);
> @@ -3132,6 +3130,7 @@ static struct inode *shmem_alloc_inode(struct super_block *sb)
> static void shmem_destroy_callback(struct rcu_head *head)
> {
> struct inode *inode = container_of(head, struct inode, i_rcu);
> + kfree(inode->i_link);
> kmem_cache_free(shmem_inode_cachep, SHMEM_I(inode));
> }
>
Powered by blists - more mailing lists