All of lore.kernel.org
 help / color / mirror / Atom feed
* binderfs interferes with syzkaller?
@ 2020-01-25 17:49 ` Dmitry Vyukov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-25 17:49 UTC (permalink / raw)
  To: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

Hi binder maintainers,

It seems that something has happened and now syzbot has 0 coverage in
drivers/android/binder.c:
https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
It covered at least something there before as it found some bugs in binder code.
I _suspect_ it may be related to introduction binderfs, but it's
purely based on the fact that binderfs changed lots of things there.
And I see it claims to be backward compatible.

syzkaller strategy to reach binder devices is to use
CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
give each test process a private one):
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671

Then it knows how to open these /dev/binderN devices:
https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
and do stuff with them.

Did these devices disappear or something?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* binderfs interferes with syzkaller?
@ 2020-01-25 17:49 ` Dmitry Vyukov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-25 17:49 UTC (permalink / raw)
  To: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

Hi binder maintainers,

It seems that something has happened and now syzbot has 0 coverage in
drivers/android/binder.c:
https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
It covered at least something there before as it found some bugs in binder code.
I _suspect_ it may be related to introduction binderfs, but it's
purely based on the fact that binderfs changed lots of things there.
And I see it claims to be backward compatible.

syzkaller strategy to reach binder devices is to use
CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
give each test process a private one):
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671

Then it knows how to open these /dev/binderN devices:
https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
and do stuff with them.

Did these devices disappear or something?
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-25 17:49 ` Dmitry Vyukov
@ 2020-01-25 18:13   ` Dmitry Vyukov
  -1 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-25 18:13 UTC (permalink / raw)
  To: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> Hi binder maintainers,
>
> It seems that something has happened and now syzbot has 0 coverage in
> drivers/android/binder.c:
> https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> It covered at least something there before as it found some bugs in binder code.
> I _suspect_ it may be related to introduction binderfs, but it's
> purely based on the fact that binderfs changed lots of things there.
> And I see it claims to be backward compatible.
>
> syzkaller strategy to reach binder devices is to use
> CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> give each test process a private one):
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
>
> Then it knows how to open these /dev/binderN devices:
> https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> and do stuff with them.
>
> Did these devices disappear or something?

Oh, I see, it's backwards compatible if it's not enabled, right?

if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
    strcmp(binder_devices_param, "") != 0) {
/*
* Copy the module_parameter string, because we don't want to
* tokenize it in-place.
*/
device_names = kstrdup(binder_devices_param, GFP_KERNEL);
if (!device_names) {
ret = -ENOMEM;
goto err_alloc_device_names_failed;
}

device_tmp = device_names;
while ((device_name = strsep(&device_tmp, ","))) {
ret = init_binder_device(device_name);
if (ret)
goto err_init_binder_device_failed;
}
}

And we enabled it because, well, enabling things generally gives more
coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to
restore coverage in the binder itself.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-25 18:13   ` Dmitry Vyukov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-25 18:13 UTC (permalink / raw)
  To: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> Hi binder maintainers,
>
> It seems that something has happened and now syzbot has 0 coverage in
> drivers/android/binder.c:
> https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> It covered at least something there before as it found some bugs in binder code.
> I _suspect_ it may be related to introduction binderfs, but it's
> purely based on the fact that binderfs changed lots of things there.
> And I see it claims to be backward compatible.
>
> syzkaller strategy to reach binder devices is to use
> CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> give each test process a private one):
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
>
> Then it knows how to open these /dev/binderN devices:
> https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> and do stuff with them.
>
> Did these devices disappear or something?

Oh, I see, it's backwards compatible if it's not enabled, right?

if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
    strcmp(binder_devices_param, "") != 0) {
/*
* Copy the module_parameter string, because we don't want to
* tokenize it in-place.
*/
device_names = kstrdup(binder_devices_param, GFP_KERNEL);
if (!device_names) {
ret = -ENOMEM;
goto err_alloc_device_names_failed;
}

device_tmp = device_names;
while ((device_name = strsep(&device_tmp, ","))) {
ret = init_binder_device(device_name);
if (ret)
goto err_init_binder_device_failed;
}
}

And we enabled it because, well, enabling things generally gives more
coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to
restore coverage in the binder itself.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-25 17:49 ` Dmitry Vyukov
@ 2020-01-26  8:55   ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-26  8:55 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Christian Brauner, Hridya Valsaraju, Arve Hjønnevåg,
	Todd Kjos, Martijn Coenen, Joel Fernandes,
	open list:ANDROID DRIVERS, LKML, syzkaller

On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> Hi binder maintainers,
> 
> It seems that something has happened and now syzbot has 0 coverage in
> drivers/android/binder.c:
> https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> It covered at least something there before as it found some bugs in binder code.
> I _suspect_ it may be related to introduction binderfs, but it's
> purely based on the fact that binderfs changed lots of things there.
> And I see it claims to be backward compatible.

It is backwards compatible if you mount binderfs, right?

> syzkaller strategy to reach binder devices is to use
> CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> give each test process a private one):
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> 
> Then it knows how to open these /dev/binderN devices:
> https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> and do stuff with them.
> 
> Did these devices disappear or something?

Try mounting binderfs and then you should be able to see them all.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-26  8:55   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kroah-Hartman @ 2020-01-26  8:55 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: open list:ANDROID DRIVERS, Todd Kjos, LKML, Joel Fernandes,
	Arve Hjønnevåg, syzkaller, Hridya Valsaraju,
	Martijn Coenen, Christian Brauner

On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> Hi binder maintainers,
> 
> It seems that something has happened and now syzbot has 0 coverage in
> drivers/android/binder.c:
> https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> It covered at least something there before as it found some bugs in binder code.
> I _suspect_ it may be related to introduction binderfs, but it's
> purely based on the fact that binderfs changed lots of things there.
> And I see it claims to be backward compatible.

It is backwards compatible if you mount binderfs, right?

> syzkaller strategy to reach binder devices is to use
> CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> give each test process a private one):
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> 
> Then it knows how to open these /dev/binderN devices:
> https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> and do stuff with them.
> 
> Did these devices disappear or something?

Try mounting binderfs and then you should be able to see them all.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-26  8:55   ` Greg Kroah-Hartman
@ 2020-01-26  9:35     ` Christian Brauner
  -1 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-26  9:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dmitry Vyukov, Christian Brauner, Hridya Valsaraju,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> > Hi binder maintainers,
> > 
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
> 
> It is backwards compatible if you mount binderfs, right?

Yes, it is backwards compatible. The devices that would usually be
created in devtmpfs are now created in binderfs. The core
binder-codepaths are the same.

Christian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-26  9:35     ` Christian Brauner
  0 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-26  9:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:ANDROID DRIVERS, Todd Kjos, LKML, Joel Fernandes,
	Arve Hjønnevåg, syzkaller, Dmitry Vyukov,
	Hridya Valsaraju, Martijn Coenen, Christian Brauner

On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> > Hi binder maintainers,
> > 
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
> 
> It is backwards compatible if you mount binderfs, right?

Yes, it is backwards compatible. The devices that would usually be
created in devtmpfs are now created in binderfs. The core
binder-codepaths are the same.

Christian
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-25 18:13   ` Dmitry Vyukov
@ 2020-01-26  9:51     ` Christian Brauner
  -1 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-26  9:51 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sat, Jan 25, 2020 at 07:13:03PM +0100, Dmitry Vyukov wrote:
> On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > Hi binder maintainers,
> >
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
> >
> > syzkaller strategy to reach binder devices is to use
> > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > give each test process a private one):
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> >
> > Then it knows how to open these /dev/binderN devices:
> > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > and do stuff with them.
> >
> > Did these devices disappear or something?
> 
> Oh, I see, it's backwards compatible if it's not enabled, right?

It's backwards compatible.
Let me give a little more detail. The legacy binder interface would
create all devices listed in the module parameter
CONFIG_ANDROID_BINDER_DEVICES. These devices were created using 
misc_register(&binder_device->miscdev);
in the host's devtmpfs mount.
If binderfs is enabled these devices are now created in the binderfs
instance instead.
For full backwards compatibility with old Android versions symlinks (or
bind mounts) can be provided in the host's devtmpfs. This is what I
recommended a few months ago.

> And we enabled it because, well, enabling things generally gives more
> coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to

I would at least try to test both somehow. It's likely that binderfs
will replace legacy binder devices so if we could keep testing it
somehow that would be great.

> restore coverage in the binder itself.

I'm not completely sure what you mean here. The binder IPC codepaths are
nearly the same. The difference is mostly in how the device information
is cast out before actual binder-ipc operations take place.
In any case, testing both would obviously be preferred but binderfs
strikes me as more worthy of testing.

Christian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-26  9:51     ` Christian Brauner
  0 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-26  9:51 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Greg Kroah-Hartman, LKML,
	Joel Fernandes, Arve Hjønnevåg, syzkaller,
	Hridya Valsaraju, Martijn Coenen, Christian Brauner

On Sat, Jan 25, 2020 at 07:13:03PM +0100, Dmitry Vyukov wrote:
> On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> >
> > Hi binder maintainers,
> >
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
> >
> > syzkaller strategy to reach binder devices is to use
> > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > give each test process a private one):
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> >
> > Then it knows how to open these /dev/binderN devices:
> > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > and do stuff with them.
> >
> > Did these devices disappear or something?
> 
> Oh, I see, it's backwards compatible if it's not enabled, right?

It's backwards compatible.
Let me give a little more detail. The legacy binder interface would
create all devices listed in the module parameter
CONFIG_ANDROID_BINDER_DEVICES. These devices were created using 
misc_register(&binder_device->miscdev);
in the host's devtmpfs mount.
If binderfs is enabled these devices are now created in the binderfs
instance instead.
For full backwards compatibility with old Android versions symlinks (or
bind mounts) can be provided in the host's devtmpfs. This is what I
recommended a few months ago.

> And we enabled it because, well, enabling things generally gives more
> coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to

I would at least try to test both somehow. It's likely that binderfs
will replace legacy binder devices so if we could keep testing it
somehow that would be great.

> restore coverage in the binder itself.

I'm not completely sure what you mean here. The binder IPC codepaths are
nearly the same. The difference is mostly in how the device information
is cast out before actual binder-ipc operations take place.
In any case, testing both would obviously be preferred but binderfs
strikes me as more worthy of testing.

Christian
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-26  8:55   ` Greg Kroah-Hartman
@ 2020-01-27 16:47     ` Todd Kjos
  -1 siblings, 0 replies; 18+ messages in thread
From: Todd Kjos @ 2020-01-27 16:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dmitry Vyukov, Christian Brauner, Hridya Valsaraju,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sun, Jan 26, 2020 at 12:55 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> > Hi binder maintainers,
> >
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
>
> It is backwards compatible if you mount binderfs, right?

If binderfs is enabled and binderfs is mounted, then it's compatible
except that the path to be opened is under the binderfs mount point
instead of /dev/binderX.

>
> > syzkaller strategy to reach binder devices is to use
> > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > give each test process a private one):
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> >
> > Then it knows how to open these /dev/binderN devices:
> > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > and do stuff with them.
> >
> > Did these devices disappear or something?
>
> Try mounting binderfs and then you should be able to see them all.
>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-27 16:47     ` Todd Kjos
  0 siblings, 0 replies; 18+ messages in thread
From: Todd Kjos @ 2020-01-27 16:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:ANDROID DRIVERS, Todd Kjos, LKML, Joel Fernandes,
	Arve Hjønnevåg, syzkaller, Dmitry Vyukov,
	Hridya Valsaraju, Martijn Coenen, Christian Brauner

On Sun, Jan 26, 2020 at 12:55 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> > Hi binder maintainers,
> >
> > It seems that something has happened and now syzbot has 0 coverage in
> > drivers/android/binder.c:
> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > It covered at least something there before as it found some bugs in binder code.
> > I _suspect_ it may be related to introduction binderfs, but it's
> > purely based on the fact that binderfs changed lots of things there.
> > And I see it claims to be backward compatible.
>
> It is backwards compatible if you mount binderfs, right?

If binderfs is enabled and binderfs is mounted, then it's compatible
except that the path to be opened is under the binderfs mount point
instead of /dev/binderX.

>
> > syzkaller strategy to reach binder devices is to use
> > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > give each test process a private one):
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> >
> > Then it knows how to open these /dev/binderN devices:
> > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > and do stuff with them.
> >
> > Did these devices disappear or something?
>
> Try mounting binderfs and then you should be able to see them all.
>
> thanks,
>
> greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-26  9:51     ` Christian Brauner
@ 2020-01-27 17:11       ` Dmitry Vyukov
  -1 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-27 17:11 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Christian Brauner, Hridya Valsaraju, Greg Kroah-Hartman,
	Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Joel Fernandes, open list:ANDROID DRIVERS, LKML, syzkaller

On Sun, Jan 26, 2020 at 10:51 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Sat, Jan 25, 2020 at 07:13:03PM +0100, Dmitry Vyukov wrote:
> > On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> > >
> > > Hi binder maintainers,
> > >
> > > It seems that something has happened and now syzbot has 0 coverage in
> > > drivers/android/binder.c:
> > > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > > It covered at least something there before as it found some bugs in binder code.
> > > I _suspect_ it may be related to introduction binderfs, but it's
> > > purely based on the fact that binderfs changed lots of things there.
> > > And I see it claims to be backward compatible.
> > >
> > > syzkaller strategy to reach binder devices is to use
> > > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > > give each test process a private one):
> > > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> > >
> > > Then it knows how to open these /dev/binderN devices:
> > > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > > and do stuff with them.
> > >
> > > Did these devices disappear or something?
> >
> > Oh, I see, it's backwards compatible if it's not enabled, right?
>
> It's backwards compatible.
> Let me give a little more detail. The legacy binder interface would
> create all devices listed in the module parameter
> CONFIG_ANDROID_BINDER_DEVICES. These devices were created using
> misc_register(&binder_device->miscdev);
> in the host's devtmpfs mount.
> If binderfs is enabled these devices are now created in the binderfs
> instance instead.
> For full backwards compatibility with old Android versions symlinks (or
> bind mounts) can be provided in the host's devtmpfs. This is what I
> recommended a few months ago.
>
> > And we enabled it because, well, enabling things generally gives more
> > coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to
>
> I would at least try to test both somehow. It's likely that binderfs
> will replace legacy binder devices so if we could keep testing it
> somehow that would be great.
>
> > restore coverage in the binder itself.
>
> I'm not completely sure what you mean here. The binder IPC codepaths are
> nearly the same. The difference is mostly in how the device information
> is cast out before actual binder-ipc operations take place.
> In any case, testing both would obviously be preferred but binderfs
> strikes me as more worthy of testing.

I've reverted CONFIG_ANDROID_BINDERFS to "is not set" for now as the
easiest option. We've got 35% coverage of binder.c again.

By "restore coverage" I meant that even if the IPC codepaths are
nearly the same tests knew to open /dev/binderN and these disappeared,
so no test could open any binder device and get any coverage in binder
code.

If anybody wants to try to test both, you are very welcome ;)
If you are hinting that I should do it, a single person can't test all
2000 subsystems in kernel, and testing each of them requires quite
some domain expertise. This really requires involvement of developers
of the code. I am sure there is lots to improve in the IPC codepaths
as well.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-27 17:11       ` Dmitry Vyukov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Vyukov @ 2020-01-27 17:11 UTC (permalink / raw)
  To: Christian Brauner
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Greg Kroah-Hartman, LKML,
	Joel Fernandes, Arve Hjønnevåg, syzkaller,
	Hridya Valsaraju, Martijn Coenen, Christian Brauner

On Sun, Jan 26, 2020 at 10:51 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Sat, Jan 25, 2020 at 07:13:03PM +0100, Dmitry Vyukov wrote:
> > On Sat, Jan 25, 2020 at 6:49 PM Dmitry Vyukov <dvyukov@google.com> wrote:
> > >
> > > Hi binder maintainers,
> > >
> > > It seems that something has happened and now syzbot has 0 coverage in
> > > drivers/android/binder.c:
> > > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> > > It covered at least something there before as it found some bugs in binder code.
> > > I _suspect_ it may be related to introduction binderfs, but it's
> > > purely based on the fact that binderfs changed lots of things there.
> > > And I see it claims to be backward compatible.
> > >
> > > syzkaller strategy to reach binder devices is to use
> > > CONFIG_ANDROID_BINDER_DEVICES to create a bunch of binderN devices (to
> > > give each test process a private one):
> > > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L5671
> > >
> > > Then it knows how to open these /dev/binderN devices:
> > > https://github.com/google/syzkaller/blob/master/sys/linux/dev_binder.txt#L22
> > > and do stuff with them.
> > >
> > > Did these devices disappear or something?
> >
> > Oh, I see, it's backwards compatible if it's not enabled, right?
>
> It's backwards compatible.
> Let me give a little more detail. The legacy binder interface would
> create all devices listed in the module parameter
> CONFIG_ANDROID_BINDER_DEVICES. These devices were created using
> misc_register(&binder_device->miscdev);
> in the host's devtmpfs mount.
> If binderfs is enabled these devices are now created in the binderfs
> instance instead.
> For full backwards compatibility with old Android versions symlinks (or
> bind mounts) can be provided in the host's devtmpfs. This is what I
> recommended a few months ago.
>
> > And we enabled it because, well, enabling things generally gives more
> > coverage. I guess I will disable CONFIG_ANDROID_BINDERFS for now to
>
> I would at least try to test both somehow. It's likely that binderfs
> will replace legacy binder devices so if we could keep testing it
> somehow that would be great.
>
> > restore coverage in the binder itself.
>
> I'm not completely sure what you mean here. The binder IPC codepaths are
> nearly the same. The difference is mostly in how the device information
> is cast out before actual binder-ipc operations take place.
> In any case, testing both would obviously be preferred but binderfs
> strikes me as more worthy of testing.

I've reverted CONFIG_ANDROID_BINDERFS to "is not set" for now as the
easiest option. We've got 35% coverage of binder.c again.

By "restore coverage" I meant that even if the IPC codepaths are
nearly the same tests knew to open /dev/binderN and these disappeared,
so no test could open any binder device and get any coverage in binder
code.

If anybody wants to try to test both, you are very welcome ;)
If you are hinting that I should do it, a single person can't test all
2000 subsystems in kernel, and testing each of them requires quite
some domain expertise. This really requires involvement of developers
of the code. I am sure there is lots to improve in the IPC codepaths
as well.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-26  9:35     ` Christian Brauner
@ 2020-01-28 13:46       ` Eric W. Biederman
  -1 siblings, 0 replies; 18+ messages in thread
From: Eric W. Biederman @ 2020-01-28 13:46 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Greg Kroah-Hartman, Dmitry Vyukov, Christian Brauner,
	Hridya Valsaraju, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, open list:ANDROID DRIVERS, LKML,
	syzkaller

Christian Brauner <christian.brauner@ubuntu.com> writes:

> On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
>> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
>> > Hi binder maintainers,
>> > 
>> > It seems that something has happened and now syzbot has 0 coverage in
>> > drivers/android/binder.c:
>> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
>> > It covered at least something there before as it found some bugs in binder code.
>> > I _suspect_ it may be related to introduction binderfs, but it's
>> > purely based on the fact that binderfs changed lots of things there.
>> > And I see it claims to be backward compatible.
>> 
>> It is backwards compatible if you mount binderfs, right?
>
> Yes, it is backwards compatible. The devices that would usually be
> created in devtmpfs are now created in binderfs. The core
> binder-codepaths are the same.

Any chance you can add code to the binderfs case to automatically
create the symlinks to the standard mount location in devtmpfs?

That way existing userspace might not need to care how the kernel is
configured.

Eric


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-28 13:46       ` Eric W. Biederman
  0 siblings, 0 replies; 18+ messages in thread
From: Eric W. Biederman @ 2020-01-28 13:46 UTC (permalink / raw)
  To: Christian Brauner
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Greg Kroah-Hartman, LKML,
	Joel Fernandes, Arve Hjønnevåg, syzkaller,
	Dmitry Vyukov, Hridya Valsaraju, Martijn Coenen,
	Christian Brauner

Christian Brauner <christian.brauner@ubuntu.com> writes:

> On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
>> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
>> > Hi binder maintainers,
>> > 
>> > It seems that something has happened and now syzbot has 0 coverage in
>> > drivers/android/binder.c:
>> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
>> > It covered at least something there before as it found some bugs in binder code.
>> > I _suspect_ it may be related to introduction binderfs, but it's
>> > purely based on the fact that binderfs changed lots of things there.
>> > And I see it claims to be backward compatible.
>> 
>> It is backwards compatible if you mount binderfs, right?
>
> Yes, it is backwards compatible. The devices that would usually be
> created in devtmpfs are now created in binderfs. The core
> binder-codepaths are the same.

Any chance you can add code to the binderfs case to automatically
create the symlinks to the standard mount location in devtmpfs?

That way existing userspace might not need to care how the kernel is
configured.

Eric

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
  2020-01-28 13:46       ` Eric W. Biederman
@ 2020-01-28 13:54         ` Christian Brauner
  -1 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-28 13:54 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg Kroah-Hartman, Dmitry Vyukov, Christian Brauner,
	Hridya Valsaraju, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, open list:ANDROID DRIVERS, LKML,
	syzkaller

On Tue, Jan 28, 2020 at 07:46:08AM -0600, Eric W. Biederman wrote:
> Christian Brauner <christian.brauner@ubuntu.com> writes:
> 
> > On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
> >> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> >> > Hi binder maintainers,
> >> > 
> >> > It seems that something has happened and now syzbot has 0 coverage in
> >> > drivers/android/binder.c:
> >> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> >> > It covered at least something there before as it found some bugs in binder code.
> >> > I _suspect_ it may be related to introduction binderfs, but it's
> >> > purely based on the fact that binderfs changed lots of things there.
> >> > And I see it claims to be backward compatible.
> >> 
> >> It is backwards compatible if you mount binderfs, right?
> >
> > Yes, it is backwards compatible. The devices that would usually be
> > created in devtmpfs are now created in binderfs. The core
> > binder-codepaths are the same.
> 
> Any chance you can add code to the binderfs case to automatically
> create the symlinks to the standard mount location in devtmpfs?

Yeah, that's certainly doable and should be fairly easy. My reasoning
for not doing it was that it would be trivial for userspace to add in
the symlinks with an init script or service file.
We can also place this CONFIG_BINDERFS_DEVTMFPS_SYMLINK (random name)
which defaults to Y. Then - if userspace decides to completely move
from /dev/binder* to /dev/binderfs/binder* nodes and doesn't need the
symlinks - they can opt out of this by setting it to N. If Todd agrees
that something like this makes for Android too then we can do this.

Christian

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: binderfs interferes with syzkaller?
@ 2020-01-28 13:54         ` Christian Brauner
  0 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2020-01-28 13:54 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Greg Kroah-Hartman, LKML,
	Joel Fernandes, Arve Hjønnevåg, syzkaller,
	Dmitry Vyukov, Hridya Valsaraju, Martijn Coenen,
	Christian Brauner

On Tue, Jan 28, 2020 at 07:46:08AM -0600, Eric W. Biederman wrote:
> Christian Brauner <christian.brauner@ubuntu.com> writes:
> 
> > On Sun, Jan 26, 2020 at 09:55:35AM +0100, Greg Kroah-Hartman wrote:
> >> On Sat, Jan 25, 2020 at 06:49:49PM +0100, Dmitry Vyukov wrote:
> >> > Hi binder maintainers,
> >> > 
> >> > It seems that something has happened and now syzbot has 0 coverage in
> >> > drivers/android/binder.c:
> >> > https://storage.googleapis.com/syzkaller/cover/ci-upstream-kasan-gce-root.html
> >> > It covered at least something there before as it found some bugs in binder code.
> >> > I _suspect_ it may be related to introduction binderfs, but it's
> >> > purely based on the fact that binderfs changed lots of things there.
> >> > And I see it claims to be backward compatible.
> >> 
> >> It is backwards compatible if you mount binderfs, right?
> >
> > Yes, it is backwards compatible. The devices that would usually be
> > created in devtmpfs are now created in binderfs. The core
> > binder-codepaths are the same.
> 
> Any chance you can add code to the binderfs case to automatically
> create the symlinks to the standard mount location in devtmpfs?

Yeah, that's certainly doable and should be fairly easy. My reasoning
for not doing it was that it would be trivial for userspace to add in
the symlinks with an init script or service file.
We can also place this CONFIG_BINDERFS_DEVTMFPS_SYMLINK (random name)
which defaults to Y. Then - if userspace decides to completely move
from /dev/binder* to /dev/binderfs/binder* nodes and doesn't need the
symlinks - they can opt out of this by setting it to N. If Todd agrees
that something like this makes for Android too then we can do this.

Christian
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-01-28 14:40 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-25 17:49 binderfs interferes with syzkaller? Dmitry Vyukov
2020-01-25 17:49 ` Dmitry Vyukov
2020-01-25 18:13 ` Dmitry Vyukov
2020-01-25 18:13   ` Dmitry Vyukov
2020-01-26  9:51   ` Christian Brauner
2020-01-26  9:51     ` Christian Brauner
2020-01-27 17:11     ` Dmitry Vyukov
2020-01-27 17:11       ` Dmitry Vyukov
2020-01-26  8:55 ` Greg Kroah-Hartman
2020-01-26  8:55   ` Greg Kroah-Hartman
2020-01-26  9:35   ` Christian Brauner
2020-01-26  9:35     ` Christian Brauner
2020-01-28 13:46     ` Eric W. Biederman
2020-01-28 13:46       ` Eric W. Biederman
2020-01-28 13:54       ` Christian Brauner
2020-01-28 13:54         ` Christian Brauner
2020-01-27 16:47   ` Todd Kjos
2020-01-27 16:47     ` Todd Kjos

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.