Re: general protection fault in snd_ctl_release

[Takashi Iwai]

On Wed, 02 Sep 2020 08:57:22 +0200,
syzbot wrote:
> 
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    b51594df Merge tag 'docs-5.9-3' of git://git.lwn.net/linux
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=172fea15900000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=3c5f6ce8d5b68299
> dashboard link: https://syzkaller.appspot.com/bug?extid=dd94e1d44f61c258d538
> compiler:       gcc (GCC) 10.1.0-syz 20200507
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=179811c1900000
> 
> The issue was bisected to:
> 
> commit a9ed4a6560b8562b7e2e2bed9527e88001f7b682
> Author: Marc Zyngier <maz@xxxxxxxxxx>
> Date:   Wed Aug 19 16:12:17 2020 +0000
> 
>     epoll: Keep a reference on files added to the check list

Luckily, this one could be easily reproduced locally, and I confirmed
that the commit above indeed brought a regression.

It seems that the same file gets closed twice after this patch, and
KASAN caught the double-free.  With the debug patch below, the syz
reproducer hits occasionally the first check point; it indicates that
we're calling get_file() to the file being deleted.  Then fput() will
be called again to this file, and it's deleted again in the end.

Marc, Al, could you guys check this bug?


Thanks!

Takashi

--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1995,9 +1995,13 @@ static int ep_loop_check_proc(void *priv, void *cookie, int call_nests)
 			 * during ep_insert().
 			 */
 			if (list_empty(&epi->ffd.file->f_tfile_llink)) {
+				if (!file_count(epi->ffd.file)) {
+					pr_err("XXX file being deleted\n");
+				} else {
 				get_file(epi->ffd.file);
 				list_add(&epi->ffd.file->f_tfile_llink,
 					 &tfile_check_list);
+				}
 			}
 		}
 	}
@@ -2205,6 +2209,8 @@ int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds,
 				error = -ELOOP;
 				if (ep_loop_check(ep, tf.file) != 0)
 					goto error_tgt_fput;
+			} else if (!file_count(tf.file)) {
+				pr_err("XXX file being deleted #2\n");
 			} else {
 				get_file(tf.file);
 				list_add(&tf.file->f_tfile_llink,