* [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
@ 2016-08-09 12:02 Julia Lawall
2016-08-15 14:54 ` Matt Fleming
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2016-08-09 12:02 UTC (permalink / raw)
To: Sylvain Chouleur, Matt Fleming, Ard Biesheuvel
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA, kbuild-all-JC7UmRfGjtg
Possibility of a double free.
julia
---------- Forwarded message ----------
Date: Tue, 9 Aug 2016 19:58:20 +0800
From: kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: kbuild-JC7UmRfGjtg@public.gmane.org
Cc: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
Subject: [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference
preceded by free on line 162
CC: kbuild-all-JC7UmRfGjtg@public.gmane.org
CC: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
TO: Sylvain Chouleur <sylvain.chouleur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
CC: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
CC: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git next
head: 302c675f2c1f6a2426709695d6dfe8683cfc7bab
commit: 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1 [15/19] efi: Don't use spinlocks for efi vars
:::::: branch date: 4 days ago
:::::: commit date: 2 weeks ago
>> fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
git remote add efi https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git
git remote update efi
git checkout 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1
vim +180 fs/efivarfs/super.c
d68772b7 Matt Fleming 2013-02-08 156 if (IS_ERR(dentry)) {
d68772b7 Matt Fleming 2013-02-08 157 err = PTR_ERR(dentry);
d68772b7 Matt Fleming 2013-02-08 158 goto fail_inode;
d68772b7 Matt Fleming 2013-02-08 159 }
d68772b7 Matt Fleming 2013-02-08 160
d68772b7 Matt Fleming 2013-02-08 161 /* copied by the above to local storage in the dentry. */
d68772b7 Matt Fleming 2013-02-08 @162 kfree(name);
d68772b7 Matt Fleming 2013-02-08 163
d68772b7 Matt Fleming 2013-02-08 164 efivar_entry_size(entry, &size);
0d22f33b Sylvain Chouleur 2016-07-15 165 err = efivar_entry_add(entry, &efivarfs_list);
0d22f33b Sylvain Chouleur 2016-07-15 166 if (err)
0d22f33b Sylvain Chouleur 2016-07-15 167 goto fail_inode;
d68772b7 Matt Fleming 2013-02-08 168
5955102c Al Viro 2016-01-22 169 inode_lock(inode);
d68772b7 Matt Fleming 2013-02-08 170 inode->i_private = entry;
d68772b7 Matt Fleming 2013-02-08 171 i_size_write(inode, size + sizeof(entry->var.Attributes));
5955102c Al Viro 2016-01-22 172 inode_unlock(inode);
d68772b7 Matt Fleming 2013-02-08 173 d_add(dentry, inode);
d68772b7 Matt Fleming 2013-02-08 174
d68772b7 Matt Fleming 2013-02-08 175 return 0;
d68772b7 Matt Fleming 2013-02-08 176
d68772b7 Matt Fleming 2013-02-08 177 fail_inode:
d68772b7 Matt Fleming 2013-02-08 178 iput(inode);
d68772b7 Matt Fleming 2013-02-08 179 fail_name:
d68772b7 Matt Fleming 2013-02-08 @180 kfree(name);
d68772b7 Matt Fleming 2013-02-08 181 fail:
d68772b7 Matt Fleming 2013-02-08 182 kfree(entry);
d68772b7 Matt Fleming 2013-02-08 183 return err;
:::::: The code at line 180 was first introduced by commit
:::::: d68772b7c83f4b518be15ae96f4827c8ed02f684 efivarfs: Move to fs/efivarfs
:::::: TO: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
:::::: CC: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
2016-08-09 12:02 [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162 Julia Lawall
@ 2016-08-15 14:54 ` Matt Fleming
[not found] ` <20160815145412.GH30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2016-08-15 14:54 UTC (permalink / raw)
To: Julia Lawall
Cc: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
kbuild-all-JC7UmRfGjtg, Sylvain Chouleur
On Tue, 09 Aug, at 02:02:39PM, Julia Lawall wrote:
> Possibility of a double free.
>
> julia
>
> ---------- Forwarded message ----------
> Date: Tue, 9 Aug 2016 19:58:20 +0800
> From: kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> To: kbuild-JC7UmRfGjtg@public.gmane.org
> Cc: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
> Subject: [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference
> preceded by free on line 162
>
> CC: kbuild-all-JC7UmRfGjtg@public.gmane.org
> CC: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> TO: Sylvain Chouleur <sylvain.chouleur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> CC: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> CC: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git next
> head: 302c675f2c1f6a2426709695d6dfe8683cfc7bab
> commit: 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1 [15/19] efi: Don't use spinlocks for efi vars
> :::::: branch date: 4 days ago
> :::::: commit date: 2 weeks ago
>
> >> fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
>
> git remote add efi https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git
> git remote update efi
> git checkout 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1
> vim +180 fs/efivarfs/super.c
>
> d68772b7 Matt Fleming 2013-02-08 156 if (IS_ERR(dentry)) {
> d68772b7 Matt Fleming 2013-02-08 157 err = PTR_ERR(dentry);
> d68772b7 Matt Fleming 2013-02-08 158 goto fail_inode;
> d68772b7 Matt Fleming 2013-02-08 159 }
> d68772b7 Matt Fleming 2013-02-08 160
> d68772b7 Matt Fleming 2013-02-08 161 /* copied by the above to local storage in the dentry. */
> d68772b7 Matt Fleming 2013-02-08 @162 kfree(name);
> d68772b7 Matt Fleming 2013-02-08 163
> d68772b7 Matt Fleming 2013-02-08 164 efivar_entry_size(entry, &size);
> 0d22f33b Sylvain Chouleur 2016-07-15 165 err = efivar_entry_add(entry, &efivarfs_list);
> 0d22f33b Sylvain Chouleur 2016-07-15 166 if (err)
> 0d22f33b Sylvain Chouleur 2016-07-15 167 goto fail_inode;
> d68772b7 Matt Fleming 2013-02-08 168
> 5955102c Al Viro 2016-01-22 169 inode_lock(inode);
> d68772b7 Matt Fleming 2013-02-08 170 inode->i_private = entry;
> d68772b7 Matt Fleming 2013-02-08 171 i_size_write(inode, size + sizeof(entry->var.Attributes));
> 5955102c Al Viro 2016-01-22 172 inode_unlock(inode);
> d68772b7 Matt Fleming 2013-02-08 173 d_add(dentry, inode);
> d68772b7 Matt Fleming 2013-02-08 174
> d68772b7 Matt Fleming 2013-02-08 175 return 0;
> d68772b7 Matt Fleming 2013-02-08 176
> d68772b7 Matt Fleming 2013-02-08 177 fail_inode:
> d68772b7 Matt Fleming 2013-02-08 178 iput(inode);
> d68772b7 Matt Fleming 2013-02-08 179 fail_name:
> d68772b7 Matt Fleming 2013-02-08 @180 kfree(name);
> d68772b7 Matt Fleming 2013-02-08 181 fail:
> d68772b7 Matt Fleming 2013-02-08 182 kfree(entry);
> d68772b7 Matt Fleming 2013-02-08 183 return err;
Indeed. How about this?
---
>From 403180deeb68ce6b78ac0e22c50dc6aa2003dab5 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Date: Mon, 15 Aug 2016 15:29:20 +0100
Subject: [PATCH] fs/efivarfs: Fix double kfree() in error path
Julia reported that we may double free 'name' in efivarfs_callback(),
and that this bug was introduced by commit 0d22f33bc37c ("efi: Don't
use spinlocks for efi vars").
Move one of the kfree()s until after the point at which we know we are
definitely on the success path.
Reported-by: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Sylvain Chouleur <sylvain.chouleur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
---
fs/efivarfs/super.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index e48feb022d82..a0837453dc2c 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -158,14 +158,14 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
goto fail_inode;
}
- /* copied by the above to local storage in the dentry. */
- kfree(name);
-
efivar_entry_size(entry, &size);
err = efivar_entry_add(entry, &efivarfs_list);
if (err)
goto fail_inode;
+ /* copied by the above to local storage in the dentry. */
+ kfree(name);
+
inode_lock(inode);
inode->i_private = entry;
i_size_write(inode, size + sizeof(entry->var.Attributes));
--
2.7.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
[not found] ` <20160815145412.GH30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
@ 2016-08-15 14:56 ` Julia Lawall
0 siblings, 0 replies; 3+ messages in thread
From: Julia Lawall @ 2016-08-15 14:56 UTC (permalink / raw)
To: Matt Fleming
Cc: Ard Biesheuvel, linux-efi-u79uwXL29TY76Z2rM5mHXA,
kbuild-all-JC7UmRfGjtg, Sylvain Chouleur
On Mon, 15 Aug 2016, Matt Fleming wrote:
> On Tue, 09 Aug, at 02:02:39PM, Julia Lawall wrote:
> > Possibility of a double free.
> >
> > julia
> >
> > ---------- Forwarded message ----------
> > Date: Tue, 9 Aug 2016 19:58:20 +0800
> > From: kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > To: kbuild-JC7UmRfGjtg@public.gmane.org
> > Cc: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
> > Subject: [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference
> > preceded by free on line 162
> >
> > CC: kbuild-all-JC7UmRfGjtg@public.gmane.org
> > CC: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > TO: Sylvain Chouleur <sylvain.chouleur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > CC: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> > CC: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git next
> > head: 302c675f2c1f6a2426709695d6dfe8683cfc7bab
> > commit: 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1 [15/19] efi: Don't use spinlocks for efi vars
> > :::::: branch date: 4 days ago
> > :::::: commit date: 2 weeks ago
> >
> > >> fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162
> >
> > git remote add efi https://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git
> > git remote update efi
> > git checkout 0d22f33bc37ce1c9f10dd304bd335d6feb7796d1
> > vim +180 fs/efivarfs/super.c
> >
> > d68772b7 Matt Fleming 2013-02-08 156 if (IS_ERR(dentry)) {
> > d68772b7 Matt Fleming 2013-02-08 157 err = PTR_ERR(dentry);
> > d68772b7 Matt Fleming 2013-02-08 158 goto fail_inode;
> > d68772b7 Matt Fleming 2013-02-08 159 }
> > d68772b7 Matt Fleming 2013-02-08 160
> > d68772b7 Matt Fleming 2013-02-08 161 /* copied by the above to local storage in the dentry. */
> > d68772b7 Matt Fleming 2013-02-08 @162 kfree(name);
> > d68772b7 Matt Fleming 2013-02-08 163
> > d68772b7 Matt Fleming 2013-02-08 164 efivar_entry_size(entry, &size);
> > 0d22f33b Sylvain Chouleur 2016-07-15 165 err = efivar_entry_add(entry, &efivarfs_list);
> > 0d22f33b Sylvain Chouleur 2016-07-15 166 if (err)
> > 0d22f33b Sylvain Chouleur 2016-07-15 167 goto fail_inode;
> > d68772b7 Matt Fleming 2013-02-08 168
> > 5955102c Al Viro 2016-01-22 169 inode_lock(inode);
> > d68772b7 Matt Fleming 2013-02-08 170 inode->i_private = entry;
> > d68772b7 Matt Fleming 2013-02-08 171 i_size_write(inode, size + sizeof(entry->var.Attributes));
> > 5955102c Al Viro 2016-01-22 172 inode_unlock(inode);
> > d68772b7 Matt Fleming 2013-02-08 173 d_add(dentry, inode);
> > d68772b7 Matt Fleming 2013-02-08 174
> > d68772b7 Matt Fleming 2013-02-08 175 return 0;
> > d68772b7 Matt Fleming 2013-02-08 176
> > d68772b7 Matt Fleming 2013-02-08 177 fail_inode:
> > d68772b7 Matt Fleming 2013-02-08 178 iput(inode);
> > d68772b7 Matt Fleming 2013-02-08 179 fail_name:
> > d68772b7 Matt Fleming 2013-02-08 @180 kfree(name);
> > d68772b7 Matt Fleming 2013-02-08 181 fail:
> > d68772b7 Matt Fleming 2013-02-08 182 kfree(entry);
> > d68772b7 Matt Fleming 2013-02-08 183 return err;
>
> Indeed. How about this?
It looks better to me.
Acked-by: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
>
> ---
>
> From 403180deeb68ce6b78ac0e22c50dc6aa2003dab5 Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> Date: Mon, 15 Aug 2016 15:29:20 +0100
> Subject: [PATCH] fs/efivarfs: Fix double kfree() in error path
>
> Julia reported that we may double free 'name' in efivarfs_callback(),
> and that this bug was introduced by commit 0d22f33bc37c ("efi: Don't
> use spinlocks for efi vars").
>
> Move one of the kfree()s until after the point at which we know we are
> definitely on the success path.
>
> Reported-by: Julia Lawall <julia.lawall-L2FTfq7BK8M@public.gmane.org>
> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Sylvain Chouleur <sylvain.chouleur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> ---
> fs/efivarfs/super.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index e48feb022d82..a0837453dc2c 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -158,14 +158,14 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
> goto fail_inode;
> }
>
> - /* copied by the above to local storage in the dentry. */
> - kfree(name);
> -
> efivar_entry_size(entry, &size);
> err = efivar_entry_add(entry, &efivarfs_list);
> if (err)
> goto fail_inode;
>
> + /* copied by the above to local storage in the dentry. */
> + kfree(name);
> +
> inode_lock(inode);
> inode->i_private = entry;
> i_size_write(inode, size + sizeof(entry->var.Attributes));
> --
> 2.7.3
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-08-15 14:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-09 12:02 [efi:next 15/19] fs/efivarfs/super.c:180:7-11: ERROR: reference preceded by free on line 162 Julia Lawall
2016-08-15 14:54 ` Matt Fleming
[not found] ` <20160815145412.GH30909-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-08-15 14:56 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox