* [PATCH] vfs: fix possible use after free in finish_open()
@ 2010-10-25 13:16 Miklos Szeredi
2010-10-26 10:28 ` Miklos Szeredi
0 siblings, 1 reply; 3+ messages in thread
From: Miklos Szeredi @ 2010-10-25 13:16 UTC (permalink / raw)
To: Al Viro; +Cc: akpm, linux-fsdevel, linux-kernel
From: Miklos Szeredi <mszeredi@suse.cz>
In finish_open() nd->path is used after nameidata_to_filp() already
released it. Fix by acquiring a ref to nd->path and releasing after
the last use.
Similar fix needed in do_last().
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: stable@kernel.org
---
fs/namei.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c 2010-10-25 15:07:17.000000000 +0200
+++ linux-2.6/fs/namei.c 2010-10-25 15:07:32.000000000 +0200
@@ -1549,11 +1549,16 @@ static struct file *finish_open(struct n
error = mnt_want_write(nd->path.mnt);
if (error)
goto exit;
+
+ /* nameidata_to_filp() puts nd->path */
+ path_get(&nd->path);
}
error = may_open(&nd->path, acc_mode, open_flag);
if (error) {
- if (will_truncate)
+ if (will_truncate) {
mnt_drop_write(nd->path.mnt);
+ path_put(&nd->path);
+ }
goto exit;
}
filp = nameidata_to_filp(nd);
@@ -1578,8 +1583,10 @@ static struct file *finish_open(struct n
* because the filp has had a write taken
* on its behalf.
*/
- if (will_truncate)
+ if (will_truncate) {
mnt_drop_write(nd->path.mnt);
+ path_put(&nd->path);
+ }
return filp;
exit:
@@ -1679,8 +1686,12 @@ static struct file *do_last(struct namei
mnt_drop_write(nd->path.mnt);
goto exit;
}
+ /* nameidata_to_filp() puts nd->path */
+ mntget(nd->path.mnt);
filp = nameidata_to_filp(nd);
mnt_drop_write(nd->path.mnt);
+ mntput(nd->path.mnt);
+ path_put(&nd->path);
if (!IS_ERR(filp)) {
error = ima_file_check(filp, acc_mode);
if (error) {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] vfs: fix possible use after free in finish_open()
2010-10-25 13:16 [PATCH] vfs: fix possible use after free in finish_open() Miklos Szeredi
@ 2010-10-26 10:28 ` Miklos Szeredi
2010-10-29 6:58 ` Al Viro
0 siblings, 1 reply; 3+ messages in thread
From: Miklos Szeredi @ 2010-10-26 10:28 UTC (permalink / raw)
To: viro; +Cc: akpm, linux-fsdevel, linux-kernel
Oops, broken patch. Here's the correct one.
----
Subject: vfs: fix possible use after free in finish_open()
From: Miklos Szeredi <mszeredi@suse.cz>
In finish_open() nd->path is used after nameidata_to_filp() already
released it. Fix by acquiring a ref to nd->path and releasing after
the last use.
Similar fix needed in do_last().
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: stable@kernel.org
---
fs/namei.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c 2010-10-26 11:29:16.000000000 +0200
+++ linux-2.6/fs/namei.c 2010-10-26 11:29:20.000000000 +0200
@@ -1549,11 +1549,16 @@ static struct file *finish_open(struct n
error = mnt_want_write(nd->path.mnt);
if (error)
goto exit;
+
+ /* nameidata_to_filp() puts nd->path */
+ path_get(&nd->path);
}
error = may_open(&nd->path, acc_mode, open_flag);
if (error) {
- if (will_truncate)
+ if (will_truncate) {
mnt_drop_write(nd->path.mnt);
+ path_put(&nd->path);
+ }
goto exit;
}
filp = nameidata_to_filp(nd);
@@ -1578,8 +1583,10 @@ static struct file *finish_open(struct n
* because the filp has had a write taken
* on its behalf.
*/
- if (will_truncate)
+ if (will_truncate) {
mnt_drop_write(nd->path.mnt);
+ path_put(&nd->path);
+ }
return filp;
exit:
@@ -1679,8 +1686,11 @@ static struct file *do_last(struct namei
mnt_drop_write(nd->path.mnt);
goto exit;
}
+ /* nameidata_to_filp() puts nd->path */
+ mntget(nd->path.mnt);
filp = nameidata_to_filp(nd);
mnt_drop_write(nd->path.mnt);
+ mntput(nd->path.mnt);
if (!IS_ERR(filp)) {
error = ima_file_check(filp, acc_mode);
if (error) {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] vfs: fix possible use after free in finish_open()
2010-10-26 10:28 ` Miklos Szeredi
@ 2010-10-29 6:58 ` Al Viro
0 siblings, 0 replies; 3+ messages in thread
From: Al Viro @ 2010-10-29 6:58 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: akpm, linux-fsdevel, linux-kernel
On Tue, Oct 26, 2010 at 12:28:48PM +0200, Miklos Szeredi wrote:
> Oops, broken patch. Here's the correct one.
>
> ----
> Subject: vfs: fix possible use after free in finish_open()
>
> From: Miklos Szeredi <mszeredi@suse.cz>
>
> In finish_open() nd->path is used after nameidata_to_filp() already
> released it. Fix by acquiring a ref to nd->path and releasing after
> the last use.
Nice catch, but I'd do it differently; that is, do not drop reference
in nameidata_to_filp() (and dup it if we do __dentry_open()) and drop
it in callers instead. Will push in a few.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-29 6:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-25 13:16 [PATCH] vfs: fix possible use after free in finish_open() Miklos Szeredi
2010-10-26 10:28 ` Miklos Szeredi
2010-10-29 6:58 ` Al Viro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).