Re: [PATCH,net-next,0/2] Improve code coverage of syzkaller
From: Willem de Bruijn Date: Wed, 20 Sep 2017 11:38:45 -0400 > I think that the compile time option was chosen because of the ns_capable > check, so that with user namespaces unprivileged processes can control this > path. Perhaps we can require capable() only to set IFF_NAPI_FRAGS. > > Then we can convert the napi_gro_receive path to be conditional on a new > IFF_NAPI flag instead of this compile time option. That works for me.
Re: [PATCH,net-next,0/2] Improve code coverage of syzkaller
On Wed, Sep 20, 2017 at 2:08 AM, David Miller wrote: > From: Petar Penkov > Date: Tue, 19 Sep 2017 21:26:14 -0700 > >> Furthermore, in a way testing already requires specific kernel >> configuration. In this particular example, syzkaller prefers >> synchronous operation and therefore needs 4KSTACKS disabled. Other >> features that require rebuilding are KASAN and dbx. From this point >> of view, I still think that having the TUN_NAPI flag has value. > > Then I think this path could be enabled/disabled with a runtime flag > just as easily, no? I think that the compile time option was chosen because of the ns_capable check, so that with user namespaces unprivileged processes can control this path. Perhaps we can require capable() only to set IFF_NAPI_FRAGS. Then we can convert the napi_gro_receive path to be conditional on a new IFF_NAPI flag instead of this compile time option.
Re: [PATCH,net-next,0/2] Improve code coverage of syzkaller
From: Petar Penkov Date: Tue, 19 Sep 2017 21:26:14 -0700 > Furthermore, in a way testing already requires specific kernel > configuration. In this particular example, syzkaller prefers > synchronous operation and therefore needs 4KSTACKS disabled. Other > features that require rebuilding are KASAN and dbx. From this point > of view, I still think that having the TUN_NAPI flag has value. Then I think this path could be enabled/disabled with a runtime flag just as easily, no?
Re: [PATCH,net-next,0/2] Improve code coverage of syzkaller
On Tue, Sep 19, 2017 at 4:01 PM, David Miller wrote: > From: Petar Penkov > Date: Tue, 19 Sep 2017 00:34:00 -0700 > >> The following patches address this by providing the user(syzkaller) >> with the ability to send via napi_gro_receive() and napi_gro_frags(). >> Additionally, syzkaller can specify how many fragments there are and >> how much data per fragment there is. This is done by exploiting the >> convenient structure of iovecs. Finally, this patch series adds >> support for exercising the flow dissector during fuzzing. >> >> The code path including napi_gro_receive() can be enabled via the >> CONFIG_TUN_NAPI compile-time flag, and can be used by users other than >> syzkaller. The remainder of the changes in this patch series give the >> user significantly more control over packets entering the kernel. To >> avoid potential security vulnerabilities, hide the ability to send >> custom skbs and the flow dissector code paths behind a run-time flag >> IFF_NAPI_FRAGS that is advertised and accepted only if CONFIG_TUN_NAPI >> is enabled. >> >> The patch series will be followed with changes to packetdrill, where >> these additions to the TUN driver are exercised and demonstrated. >> This will give the ability to write regression tests for specific >> parts of the early networking stack. >> >> Patch 1/ Add NAPI struct per receive queue, enable NAPI, and use >>napi_gro_receive() >> Patch 2/ Use NAPI skb and napi_gro_frags(), exercise flow >>dissector, and allow custom skbs. > > I'm happy with everything except the TUN_NAPI Kconfig knob > requirement. > > Rebuilding something just to test things isn't going to fly very well. > > Please make it secure somehow, enable this stuff by default. > > Thanks. Without a compile-time option, the TUN/TAP driver will have a code-path that allows user control over kernel memory allocation, and specifically over the SKBs that enter the kernel. That path might be hard to exploit as it requires some user privileges, but it does exist and increases attack surface of the kernel. While the flag certainly inconveniences testing, I think the layer of security it adds outweighs its disadvantages. Furthermore, in a way testing already requires specific kernel configuration. In this particular example, syzkaller prefers synchronous operation and therefore needs 4KSTACKS disabled. Other features that require rebuilding are KASAN and dbx. From this point of view, I still think that having the TUN_NAPI flag has value.
Re: [PATCH,net-next,0/2] Improve code coverage of syzkaller
From: Petar Penkov Date: Tue, 19 Sep 2017 00:34:00 -0700 > The following patches address this by providing the user(syzkaller) > with the ability to send via napi_gro_receive() and napi_gro_frags(). > Additionally, syzkaller can specify how many fragments there are and > how much data per fragment there is. This is done by exploiting the > convenient structure of iovecs. Finally, this patch series adds > support for exercising the flow dissector during fuzzing. > > The code path including napi_gro_receive() can be enabled via the > CONFIG_TUN_NAPI compile-time flag, and can be used by users other than > syzkaller. The remainder of the changes in this patch series give the > user significantly more control over packets entering the kernel. To > avoid potential security vulnerabilities, hide the ability to send > custom skbs and the flow dissector code paths behind a run-time flag > IFF_NAPI_FRAGS that is advertised and accepted only if CONFIG_TUN_NAPI > is enabled. > > The patch series will be followed with changes to packetdrill, where > these additions to the TUN driver are exercised and demonstrated. > This will give the ability to write regression tests for specific > parts of the early networking stack. > > Patch 1/ Add NAPI struct per receive queue, enable NAPI, and use >napi_gro_receive() > Patch 2/ Use NAPI skb and napi_gro_frags(), exercise flow >dissector, and allow custom skbs. I'm happy with everything except the TUN_NAPI Kconfig knob requirement. Rebuilding something just to test things isn't going to fly very well. Please make it secure somehow, enable this stuff by default. Thanks.
[PATCH,net-next,0/2] Improve code coverage of syzkaller
The following patches address this by providing the user(syzkaller) with the ability to send via napi_gro_receive() and napi_gro_frags(). Additionally, syzkaller can specify how many fragments there are and how much data per fragment there is. This is done by exploiting the convenient structure of iovecs. Finally, this patch series adds support for exercising the flow dissector during fuzzing. The code path including napi_gro_receive() can be enabled via the CONFIG_TUN_NAPI compile-time flag, and can be used by users other than syzkaller. The remainder of the changes in this patch series give the user significantly more control over packets entering the kernel. To avoid potential security vulnerabilities, hide the ability to send custom skbs and the flow dissector code paths behind a run-time flag IFF_NAPI_FRAGS that is advertised and accepted only if CONFIG_TUN_NAPI is enabled. The patch series will be followed with changes to packetdrill, where these additions to the TUN driver are exercised and demonstrated. This will give the ability to write regression tests for specific parts of the early networking stack. Patch 1/ Add NAPI struct per receive queue, enable NAPI, and use napi_gro_receive() Patch 2/ Use NAPI skb and napi_gro_frags(), exercise flow dissector, and allow custom skbs. Petar Penkov (2): tun: enable NAPI for TUN/TAP driver tun: enable napi_gro_frags() for TUN/TAP driver drivers/net/Kconfig | 8 ++ drivers/net/tun.c | 251 +--- include/uapi/linux/if_tun.h | 1 + 3 files changed, 246 insertions(+), 14 deletions(-) -- 2.11.0
