Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

[Hillf Danton]

On Mon, 01 Aug 2022 00:16:43 +0530 Siddh Raman Pant wrote:
> On Sun, 31 Jul 2022 23:41:31 +0530  Dipanjan Das <mail.dipanjan.das@gmail.com> wrote:
> > On Wed, Jul 27, 2022 at 09:50:52PM +0530, Siddh Raman Pant wrote:
> > > Thank you for explaining it!
> > >=20
> > > I will send a v3. Should I add a Suggested-by tag mentioning you?
> >=20
> > Sorry for jumping in.
> >=20
> > We have reported the same bug in kernel v5.10.131 [https://lore.kernel.or=
> g/all/CANX2M5bHye2ZEEhEV6PUj1kYL2KdWYeJtgXw8KZRzwrNpLYz+A@mail.gmail.com]. =
> We have been suggested to join this discussion so that we can have appropri=
> ate meta-information injected in this patch=E2=80=99s commit message to mak=
> e sure that it gets backported to v5.10.y.  Therefore, we would like to be =
> in the loop so that we can offer help in the process, if needed.
> >=20
> 
> As you are suggesting for backporting, I should CC the stable list, or mail
> after it gets merged. You have reproduced it on v5.10, but the change seems=
>  to
> be introduced by c73be61cede5 ("pipe: Add general notification queue suppor=
> t"),
> which got in at v5.8. So should it be backported till v5.8 instead?
> 
> I actually looked this up on the internet / lore now for any other reports,=
>  and
> it seems this fixes a CVE (CVE-2022-1882).
> 
> The reporter of CVE seems to have linked his patch as a part of CVE report,=
>  of
> which he sent v2, but he seems to do it in a roundabout way, and also in a =
> way
> similar to what Hillf Danton had replied to my v2 patch, wherein he missed
> 353f7988dd84 ("watchqueue: make sure to serialize 'wqueue->defunct' properl=
> y"),
> so I guess I can propose my patch as a fix for the CVE.

What is not clear is what you are fixing, with CVE-2022-1882 put aside,
given the mainline tree survived the syzbot test [1] irrespective of
other fixing efforts [2, 3].

Hillf

[1] https://lore.kernel.org/lkml/000000000000c7a83905e52bd127@google.com/

	syzbot has tested the proposed patch and the reproducer did not trigger any issue:
	
	Reported-and-tested-by: syzbot+c70d87ac1d001f29a058@syzkaller.appspotmail.com
	
	Tested on:
	
	commit:         3d7cb6b0 Linux 5.19
	git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
	console output: https://syzkaller.appspot.com/x/log.txt?x=14066d7a080000
	kernel config:  https://syzkaller.appspot.com/x/.config?x=70dd99d568a89e0
	dashboard link: https://syzkaller.appspot.com/bug?extid=c70d87ac1d001f29a058
	compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
	
	Note: no patches were applied.
	Note: testing is done by a robot and is best-effort only.

[2] https://lore.kernel.org/lkml/0000000000000dac0205e479ea39@google.com/
[3] https://lore.kernel.org/lkml/00000000000014c7ad05e4d535fc@google.com/

> 
> Note: I have already sent the v3, so please suggest any new improvements et=
> c.
> (except replying to the conversation here) to the v3, which can be found he=
> re:
> https://lore.kernel.org/linux-kernel/20220728155121.12145-1-code@siddh.me/
> 
> Also, you may want to break text into multiples lines instead of one huge l=
> ine.
> 
> Thanks,
> Siddh

Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

[Siddh Raman Pant]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

I don't know why you would send this again, thiswas already replied here:https://lore.kernel.org/linux-kernel-mentees/18259769e5e.52eb2082293078.3991591702430862151@siddh.me/Thanks,Siddh

[-- Attachment #2: Type: text/html, Size: 519 bytes --]

Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

[Siddh Raman Pant]

I don't know why you would send this again, this
was already replied here:

https://lore.kernel.org/linux-kernel-mentees/18259769e5e.52eb2082293078.3991591702430862151@siddh.me/

Thanks,
Siddh

Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

[Siddh Raman Pant]

On Mon, 01 Aug 2022 17:45:13 +0530  Hillf Danton <hdanton@sina.com> wrote:
> What is not clear is what you are fixing, with CVE-2022-1882 put aside,
> given the mainline tree survived the syzbot test [1] irrespective of
> other fixing efforts [2, 3].
> 
> Hillf
> 
> [1] https://lore.kernel.org/lkml/000000000000c7a83905e52bd127@google.com/
> 
> //	syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> //	
> //	Reported-and-tested-by: syzbot+c70d87ac1d001f29a058@syzkaller.appspotmail.com
> //	
> //	Tested on:
> //	
> //	commit:         3d7cb6b0 Linux 5.19
> //	git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> //	console output: https://syzkaller.appspot.com/x/log.txt?x=14066d7a080000
> //	kernel config:  https://syzkaller.appspot.com/x/.config?x=70dd99d568a89e0
> //	dashboard link: https://syzkaller.appspot.com/bug?extid=c70d87ac1d001f29a058
> //	compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> //	
> //	Note: no patches were applied.
> //	Note: testing is done by a robot and is best-effort only.
> 
> [2] https://lore.kernel.org/lkml/0000000000000dac0205e479ea39@google.com/
> 
> [3] https://lore.kernel.org/lkml/00000000000014c7ad05e4d535fc@google.com/
> 

(Fixed broken formatting)

This bug is about watch_queue still having a reference to a freed pipe,
which was being accessed by post_one_notification() at the time of when
I posted the v1 patch for fixing it on 23rd July, by removing the
reference to the freed pipe in the watch_queue.

Given ref. [3] by you leads to a bug about UAF in __post_watch_notification():
https://syzkaller.appspot.com/bug?extid=03d7b43290037d1f87ca

That bug is fixed by the following commit by David Howells on 28th July:
e64ab2dbd882 ("watch_queue: Fix missing locking in add_watch_to_object()")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e64ab2dbd882933b65cd82ff6235d705ad65dbb6

Given ref. [2] by you is of a patch tested by you, which can be found below:
https://groups.google.com/g/syzkaller-bugs/c/RbmAFTAIuyY/m/-vMjf-BXAQAJ

This had overlooked the existing serialization of wqueue->defunct, which
you had yourself pointed out in the reply to v2, which can be found below:
https://lore.kernel.org/linux-kernel-mentees/20220724071958.2557-1-hdanton@sina.com/

Given ref. [1] by you is about a syzbot test which was ran today, which no
longer triggers the issue. This probably happens due to the commit by David
Howells referenced earlier by me. While it does cause the reproducer to fail,
it doesn't really fix the particular issue concerned by this patch, which is
that the watch_queue has a reference to a freed pipe, which had caused a UAF.

Hope everything is clear.

Thanks,
Siddh

Re: [PATCH] kernel/watch_queue: Make pipe NULL while clearing watch_queue

[Hillf Danton]

On Mon, 01 Aug 2022 00:16:43 +0530 Siddh Raman Pant wrote:> On Sun, 31 Jul 2022 23:41:31 +0530  Dipanjan Das <mail.dipanjan.das@gmail.com> wrote:> > On Wed, Jul 27, 2022 at 09:50:52PM +0530, Siddh Raman Pant wrote:> > > Thank you for explaining it!> > >=20> > > I will send a v3. Should I add a Suggested-by tag mentioning you?> >=20> > Sorry for jumping in.> >=20> > We have reported the same bug in kernel v5.10.131 [https://lore.kernel.or=> g/all/CANX2M5bHye2ZEEhEV6PUj1kYL2KdWYeJtgXw8KZRzwrNpLYz+A@mail.gmail.com]. => We have been suggested to join this discussion so that we can have appropri=> ate meta-information injected in this patch=E2=80=99s commit message to mak=> e sure that it gets backported to v5.10.y.  Therefore, we would like to be => in the loop so that we can offer help in the process, if needed.> >=20> > As you are suggesting for backporting, I should CC the stable list, or mail> after it gets merged. You have reproduced it on v5.10, but the change seems=>  to> be introduced by c73be61cede5 ("pipe: Add general notification queue suppor=> t"),> which got in at v5.8. So should it be backported till v5.8 instead?> > I actually looked this up on the internet / lore now for any other reports,=>  and> it seems this fixes a CVE (CVE-2022-1882).> > The reporter of CVE seems to have linked his patch as a part of CVE report,=>  of> which he sent v2, but he seems to do it in a roundabout way, and also in a => way> similar to what Hillf Danton had replied to my v2 patch, wherein he missed> 353f7988dd84 ("watchqueue: make sure to serialize 'wqueue->defunct' properl=> y"),> so I guess I can propose my patch as a fix for the CVE.
What is not clear is what you are fixing, with CVE-2022-1882 put aside,given the mainline tree survived the syzbot test [1] irrespective ofother fixing efforts [2, 3].
Hillf
[1] https://lore.kernel.org/lkml/000000000000c7a83905e52bd127@google.com/
//	syzbot has tested the proposed patch and the reproducer did not trigger any issue://	//	Reported-and-tested-by: syzbot+c70d87ac1d001f29a058@syzkaller.appspotmail.com//	//	Tested on://	//	commit:         3d7cb6b0 Linux 5.19//	git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master//	console output: https://syzkaller.appspot.com/x/log.txt?x=14066d7a080000//	kernel config:  https://syzkaller.appspot.com/x/.config?x=70dd99d568a89e0//	dashboard link: https://syzkaller.appspot.com/bug?extid=c70d87ac1d001f29a058//	compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2//	//	Note: no patches were applied.//	Note: testing is done by a robot and is best-effort only.
[2] https://lore.kernel.org/lkml/0000000000000dac0205e479ea39@google.com/[3] https://lore.kernel.org/lkml/00000000000014c7ad05e4d535fc@google.com/
> > Note: I have already sent the v3, so please suggest any new improvements et=> c.> (except replying to the conversation here) to the v3, which can be found he=> re:> https://lore.kernel.org/linux-kernel/20220728155121.12145-1-code@siddh.me/> > Also, you may want to break text into multiples lines instead of one huge l=> ine.> > Thanks,> Siddh