On Mon, Jul 12, 2021 at 9:12 PM syzbot
[off-list ref] wrote:
syzbot has found a reproducer for the following issue on:
Hmm.
This issue is reported to have been already fixed:
Fix commit: 9b5b8722 file: fix close_range() for unshare+cloexec
and that fix is already in the reported HEAD commit:
HEAD commit: 7fef2edf sd: don't mess with SD_MINORS for CONFIG_DEBUG_BL..
and the oops report clearly is from that:
CPU: 1 PID: 8445 Comm: syz-executor493 Not tainted 5.14.0-rc1-syzkaller #0
so the alleged fix is already there.
So clearly commit 9b5b872215fe ("file: fix close_range() for
unshare+cloexec") does *NOT* fix the issue.
This was originally bisected to that 582f1fb6b721 ("fs, close_range:
add flag CLOSE_RANGE_CLOEXEC") in
https://syzkaller.appspot.com/bug?id=1bef50bdd9622a1969608d1090b2b4a588d0c6ac
which is where the "fix" is from.
It would probably be good if sysbot made this kind of "hey, it was
reported fixed, but it's not" very clear.
The KASAN report looks like a use-after-free, and that "use" is
actually the sanity check that the file count is non-zero, so it's
really a "struct file *" that has already been free'd.
That bogus free is a regular close() system call
But it has apparently already been closed from a workqueue:
Freed by task 8445:
__fput+0x288/0x920 fs/file_table.c:280
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
So it's some kind of confusion and re-use of a struct file pointer.
Which is certainly consistent with the "fix" in 9b5b872215fe ("file:
fix close_range() for unshare+cloexec"), but it very much looks like
that fix was incomplete and not the full story.
Some fdtable got re-allocated? The fix that wasn't a fix ends up
re-checking the maximum file number under the file_lock, but there's
clearly something else going on too.
Christian?
Linus
From: Christian Brauner <hidden> Date: 2021-07-14 07:59:34
On Tue, Jul 13, 2021 at 11:49:14AM -0700, Linus Torvalds wrote:
On Mon, Jul 12, 2021 at 9:12 PM syzbot
[off-list ref] wrote:
quoted
syzbot has found a reproducer for the following issue on:
Hmm.
This issue is reported to have been already fixed:
Fix commit: 9b5b8722 file: fix close_range() for unshare+cloexec
and that fix is already in the reported HEAD commit:
quoted
HEAD commit: 7fef2edf sd: don't mess with SD_MINORS for CONFIG_DEBUG_BL..
and the oops report clearly is from that:
quoted
CPU: 1 PID: 8445 Comm: syz-executor493 Not tainted 5.14.0-rc1-syzkaller #0
so the alleged fix is already there.
So clearly commit 9b5b872215fe ("file: fix close_range() for
unshare+cloexec") does *NOT* fix the issue.
This was originally bisected to that 582f1fb6b721 ("fs, close_range:
add flag CLOSE_RANGE_CLOEXEC") in
https://syzkaller.appspot.com/bug?id=1bef50bdd9622a1969608d1090b2b4a588d0c6ac
which is where the "fix" is from.
It would probably be good if sysbot made this kind of "hey, it was
reported fixed, but it's not" very clear.
The KASAN report looks like a use-after-free, and that "use" is
actually the sanity check that the file count is non-zero, so it's
really a "struct file *" that has already been free'd.
That bogus free is a regular close() system call
But it has apparently already been closed from a workqueue:
quoted
Freed by task 8445:
__fput+0x288/0x920 fs/file_table.c:280
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
So it's some kind of confusion and re-use of a struct file pointer.
Which is certainly consistent with the "fix" in 9b5b872215fe ("file:
fix close_range() for unshare+cloexec"), but it very much looks like
that fix was incomplete and not the full story.
Some fdtable got re-allocated? The fix that wasn't a fix ends up
re-checking the maximum file number under the file_lock, but there's
clearly something else going on too.
Christian?
Looking into this now.
I have to say I'm very confused by the syzkaller report here.
If I go to
https://syzkaller.appspot.com/bug?extid=283ce5a46486d6acdbaf
which is the original link in the report it shows me
android-54 KASAN: use-after-free Read in filp_close C 2 183d 183d 0/1 upstream: reported C repro on 2021/01/11 12:38
which seems to indicate that this happened on an Android specific 5.4
kernel?
But ok, so I click on the link "upstream: reported C repro on 2021/01/11 12:38"
which takes me to a google group
https://groups.google.com/g/syzkaller-android-bugs/c/FQj0qcRSy_M/m/wrY70QFzBAAJ
which again strongly indicates that this is an Android specific kernel?
HEAD commit: c9951e5d Merge 5.4.88 into android12-5.4
git tree: android12-5.4
but then I can click on the dashboard link for that crash report and it
takes me to:
https://syzkaller.appspot.com/bug?extid=53897bcb31b82c7a08fe
which seems to be the upstream report?
So I'm a bit confused whether I'm even looking at the correct bug report
but I'll just give the repro a try and see what's going on.
Christian
From: Christian Brauner <hidden> Date: 2021-07-14 09:14:56
On Wed, Jul 14, 2021 at 09:59:25AM +0200, Christian Brauner wrote:
On Tue, Jul 13, 2021 at 11:49:14AM -0700, Linus Torvalds wrote:
quoted
On Mon, Jul 12, 2021 at 9:12 PM syzbot
[off-list ref] wrote:
quoted
syzbot has found a reproducer for the following issue on:
Hmm.
This issue is reported to have been already fixed:
Fix commit: 9b5b8722 file: fix close_range() for unshare+cloexec
and that fix is already in the reported HEAD commit:
quoted
HEAD commit: 7fef2edf sd: don't mess with SD_MINORS for CONFIG_DEBUG_BL..
and the oops report clearly is from that:
quoted
CPU: 1 PID: 8445 Comm: syz-executor493 Not tainted 5.14.0-rc1-syzkaller #0
so the alleged fix is already there.
So clearly commit 9b5b872215fe ("file: fix close_range() for
unshare+cloexec") does *NOT* fix the issue.
This was originally bisected to that 582f1fb6b721 ("fs, close_range:
add flag CLOSE_RANGE_CLOEXEC") in
https://syzkaller.appspot.com/bug?id=1bef50bdd9622a1969608d1090b2b4a588d0c6ac
which is where the "fix" is from.
It would probably be good if sysbot made this kind of "hey, it was
reported fixed, but it's not" very clear.
The KASAN report looks like a use-after-free, and that "use" is
actually the sanity check that the file count is non-zero, so it's
really a "struct file *" that has already been free'd.
That bogus free is a regular close() system call
But it has apparently already been closed from a workqueue:
quoted
Freed by task 8445:
__fput+0x288/0x920 fs/file_table.c:280
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
So it's some kind of confusion and re-use of a struct file pointer.
Which is certainly consistent with the "fix" in 9b5b872215fe ("file:
fix close_range() for unshare+cloexec"), but it very much looks like
that fix was incomplete and not the full story.
Some fdtable got re-allocated? The fix that wasn't a fix ends up
re-checking the maximum file number under the file_lock, but there's
clearly something else going on too.
Christian?
Looking into this now.
I have to say I'm very confused by the syzkaller report here.
If I go to
https://syzkaller.appspot.com/bug?extid=283ce5a46486d6acdbaf
which is the original link in the report it shows me
android-54 KASAN: use-after-free Read in filp_close C 2 183d 183d 0/1 upstream: reported C repro on 2021/01/11 12:38
which seems to indicate that this happened on an Android specific 5.4
kernel?
But ok, so I click on the link "upstream: reported C repro on 2021/01/11 12:38"
which takes me to a google group
https://groups.google.com/g/syzkaller-android-bugs/c/FQj0qcRSy_M/m/wrY70QFzBAAJ
which again strongly indicates that this is an Android specific kernel?
HEAD commit: c9951e5d Merge 5.4.88 into android12-5.4
git tree: android12-5.4
but then I can click on the dashboard link for that crash report and it
takes me to:
https://syzkaller.appspot.com/bug?extid=53897bcb31b82c7a08fe
which seems to be the upstream report?
So I'm a bit confused whether I'm even looking at the correct bug report
but I'll just give the repro a try and see what's going on.
Ok, reproduced and I think I found the issue. It's not related to
close_fd((), I think it's caused by a UAF when FSCONFIG_SET_FD is with
the key "source" and a valid fd passed through "aux".
Briefly, fs_parameter is a union:
struct fs_parameter {
const char *key; /* Parameter name */
enum fs_value_type type:8; /* The type of value here */
union {
char *string;
void *blob;
struct filename *name;
struct file *file;
};
size_t size;
int dirfd;
};
and cgroup1_parse_param is copying out param->string when the param's
key is "source" without verifying that param->type is actually
fs_value_is_string.
I'll explain in detail in the commit once I've confirmed and tested that
my suspicion is correct.
Christian
On Wed, 14 Jul 2021 at 09:59, Christian Brauner
[off-list ref] wrote:
On Tue, Jul 13, 2021 at 11:49:14AM -0700, Linus Torvalds wrote:
quoted
On Mon, Jul 12, 2021 at 9:12 PM syzbot
[off-list ref] wrote:
quoted
syzbot has found a reproducer for the following issue on:
Hmm.
This issue is reported to have been already fixed:
Fix commit: 9b5b8722 file: fix close_range() for unshare+cloexec
and that fix is already in the reported HEAD commit:
quoted
HEAD commit: 7fef2edf sd: don't mess with SD_MINORS for CONFIG_DEBUG_BL..
and the oops report clearly is from that:
quoted
CPU: 1 PID: 8445 Comm: syz-executor493 Not tainted 5.14.0-rc1-syzkaller #0
so the alleged fix is already there.
So clearly commit 9b5b872215fe ("file: fix close_range() for
unshare+cloexec") does *NOT* fix the issue.
This was originally bisected to that 582f1fb6b721 ("fs, close_range:
add flag CLOSE_RANGE_CLOEXEC") in
https://syzkaller.appspot.com/bug?id=1bef50bdd9622a1969608d1090b2b4a588d0c6ac
which is where the "fix" is from.
It would probably be good if sysbot made this kind of "hey, it was
reported fixed, but it's not" very clear.
The KASAN report looks like a use-after-free, and that "use" is
actually the sanity check that the file count is non-zero, so it's
really a "struct file *" that has already been free'd.
That bogus free is a regular close() system call
But it has apparently already been closed from a workqueue:
quoted
Freed by task 8445:
__fput+0x288/0x920 fs/file_table.c:280
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
So it's some kind of confusion and re-use of a struct file pointer.
Which is certainly consistent with the "fix" in 9b5b872215fe ("file:
fix close_range() for unshare+cloexec"), but it very much looks like
that fix was incomplete and not the full story.
Some fdtable got re-allocated? The fix that wasn't a fix ends up
re-checking the maximum file number under the file_lock, but there's
clearly something else going on too.
Christian?
Looking into this now.
I have to say I'm very confused by the syzkaller report here.
If I go to
https://syzkaller.appspot.com/bug?extid=283ce5a46486d6acdbaf
which is the original link in the report it shows me
android-54 KASAN: use-after-free Read in filp_close C 2 183d 183d 0/1 upstream: reported C repro on 2021/01/11 12:38
which seems to indicate that this happened on an Android specific 5.4
kernel?
Hi Christian,
That's "similar bugs" section. In this section syzbot shows previous
similar bugs and similar bugs on other kernels (lts, android).
"KASAN: use-after-free Read in filp_close" also happened on Android
tree, if you click on the link, you can see crashes and reproducers on
the android tree:
https://syzkaller.appspot.com/bug?id=255edc9d4f00a881d3bf68b87d09a8b7843409e7
If you are interested only in upstream crashes/reproducers, then
ignore the "similar bugs" section.
But ok, so I click on the link "upstream: reported C repro on 2021/01/11 12:38"
which takes me to a google group
https://groups.google.com/g/syzkaller-android-bugs/c/FQj0qcRSy_M/m/wrY70QFzBAAJ
which again strongly indicates that this is an Android specific kernel?
HEAD commit: c9951e5d Merge 5.4.88 into android12-5.4
git tree: android12-5.4
but then I can click on the dashboard link for that crash report and it
takes me to:
https://syzkaller.appspot.com/bug?extid=53897bcb31b82c7a08fe
which seems to be the upstream report?
So I'm a bit confused whether I'm even looking at the correct bug report
but I'll just give the repro a try and see what's going on.
Christian
--
You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20210714075925.jtlfrhhuj4bzff3m%40wittgenstein.