| Message ID | 20200319174050.759794-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: tulip: add checks to avoid OOB access | expand |
On 2020/3/20 上午1:40, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Hello, > > * This series adds checks to avoid potential OOB access and infinite loop > issues while processing rx/tx data. > > * Tulip tx descriptors are capped at 128 to avoid infinite loop in > tulip_xmit_list_update(), wrt Tulip kernel driver > -> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/dec/tulip/tulip.h#n319 > > * Update v3: add .can_receive routine > -> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06275.html > > * Update v4: flush queued packets once they are received > -> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg05868.html > > * Update v5: fixed a typo in patch commit message > -> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06209.html > > Thank you. Looks good to me. Qiang, any change to give a test with your reproducer? Thanks > -- > Prasad J Pandit (3): > net: tulip: check frame size and r/w data length > net: tulip: add .can_receive routine > net: tulip: flush queued packets post receive > > hw/net/tulip.c | 51 +++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 9 deletions(-) > > -- > 2.25.1 > >
On 2020/3/23 上午11:43, Jason Wang wrote: > > On 2020/3/20 上午1:40, P J P wrote: >> From: Prasad J Pandit <pjp@fedoraproject.org> >> >> Hello, >> >> * This series adds checks to avoid potential OOB access and infinite >> loop >> issues while processing rx/tx data. >> >> * Tulip tx descriptors are capped at 128 to avoid infinite loop in >> tulip_xmit_list_update(), wrt Tulip kernel driver >> -> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/dec/tulip/tulip.h#n319 >> >> * Update v3: add .can_receive routine >> -> >> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06275.html >> >> * Update v4: flush queued packets once they are received >> -> >> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg05868.html >> >> * Update v5: fixed a typo in patch commit message >> -> >> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06209.html >> >> Thank you. > > > Looks good to me. > > Qiang, any change to give a test with your reproducer? > > Thanks Ok, I get this: hw/net/tulip.c:305:20: error: initialization of ‘_Bool (*)(NetClientState *)’ {aka ‘_Bool (*)(struct NetClientState *)’} from incompatible pointer type ‘int (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’} [-Werror=incompatible-pointer-types] .can_receive = tulip_can_receive, ^~~~~~~~~~~~~~~~~ Prasad, please fix this and post a new version. While at it, I prefer to squash patch 3 into patch 2 since patch 3 fixes the issue introduced by patch 2. Thanks > > >> -- >> Prasad J Pandit (3): >> net: tulip: check frame size and r/w data length >> net: tulip: add .can_receive routine >> net: tulip: flush queued packets post receive >> >> hw/net/tulip.c | 51 +++++++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 42 insertions(+), 9 deletions(-) >> >> -- >> 2.25.1 >> >>
+-- On Mon, 23 Mar 2020, Jason Wang wrote --+
| hw/net/tulip.c:305:20: error: initialization of ‘_Bool (*)(NetClientState *)’
| {aka ‘_Bool (*)(struct NetClientState *)’} from incompatible pointer type ‘int
| (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’}
| [-Werror=incompatible-pointer-types]
| .can_receive = tulip_can_receive,
| ^~~~~~~~~~~~~~~~~
Strange, I did not get it.
| Prasad, please fix this and post a new version.
|
| While at it, I prefer to squash patch 3 into patch 2 since patch 3 fixes the
| issue introduced by patch 2.
Okay, sending your way shortly.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
+-- On Mon, 23 Mar 2020, P J P wrote --+
| +-- On Mon, 23 Mar 2020, Jason Wang wrote --+
| | hw/net/tulip.c:305:20: error: initialization of ‘_Bool (*)(NetClientState *)’
| | {aka ‘_Bool (*)(struct NetClientState *)’} from incompatible pointer type ‘int
| | (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’}
| | [-Werror=incompatible-pointer-types]
| | .can_receive = tulip_can_receive,
| | ^~~~~~~~~~~~~~~~~
|
| Strange, I did not get it.
qemu/include/net.h:
typedef int (NetCanReceive)(NetClientState *);
typedef struct NetClientInfo {
...
NetCanReceive *can_receive;
...
}
@Jason,
Looking at the definition above, 'NetCanReceive' is returning an 'int' type.
When I change 'tulip_can_receive' to return a 'bool', I get the reverse error
hw/net/tulip.c:305:20: error: initialization of ‘int (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’} from incompatible pointer type ‘_Bool (*)(NetClientState *)’ {aka ‘_Bool (*)(struct NetClientState *)’}
[-Werror=incompatible-pointer-types]
305 | .can_receive = tulip_can_receive,
| ^~~~~~~~~~~~~~~~~
Maybe because of a stagged local change in your tree? (to confirm)
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Jason Wang <jasowang@redhat.com> 于2020年3月23日周一 上午11:44写道： > > On 2020/3/20 上午1:40, P J P wrote: > > From: Prasad J Pandit <pjp@fedoraproject.org> > > > > Hello, > > > > * This series adds checks to avoid potential OOB access and infinite loop > > issues while processing rx/tx data. > > > > * Tulip tx descriptors are capped at 128 to avoid infinite loop in > > tulip_xmit_list_update(), wrt Tulip kernel driver > > -> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/dec/tulip/tulip.h#n319 > > > > * Update v3: add .can_receive routine > > -> > https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06275.html > > > > * Update v4: flush queued packets once they are received > > -> > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg05868.html > > > > * Update v5: fixed a typo in patch commit message > > -> > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06209.html > > > > Thank you. > > > Looks good to me. > > Qiang, any change to give a test with your reproducer? > > Hi Jason, Sorry for missing discussing this thread as so busy these days/weeks. I will try to test this patch asap. Thanks, Li Qiang > Thanks > > > > -- > > Prasad J Pandit (3): > > net: tulip: check frame size and r/w data length > > net: tulip: add .can_receive routine > > net: tulip: flush queued packets post receive > > > > hw/net/tulip.c | 51 +++++++++++++++++++++++++++++++++++++++++--------- > > 1 file changed, 42 insertions(+), 9 deletions(-) > > > > -- > > 2.25.1 > > > > > > >
On 2020/3/23 下午1:40, P J P wrote: > +-- On Mon, 23 Mar 2020, P J P wrote --+ > | +-- On Mon, 23 Mar 2020, Jason Wang wrote --+ > | | hw/net/tulip.c:305:20: error: initialization of ‘_Bool (*)(NetClientState *)’ > | | {aka ‘_Bool (*)(struct NetClientState *)’} from incompatible pointer type ‘int > | | (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’} > | | [-Werror=incompatible-pointer-types] > | | .can_receive = tulip_can_receive, > | | ^~~~~~~~~~~~~~~~~ > | > | Strange, I did not get it. > > qemu/include/net.h: > > typedef int (NetCanReceive)(NetClientState *); > > typedef struct NetClientInfo { > ... > NetCanReceive *can_receive; > ... > } > > @Jason, > Looking at the definition above, 'NetCanReceive' is returning an 'int' type. > When I change 'tulip_can_receive' to return a 'bool', I get the reverse error > > hw/net/tulip.c:305:20: error: initialization of ‘int (*)(NetClientState *)’ {aka ‘int (*)(struct NetClientState *)’} from incompatible pointer type ‘_Bool (*)(NetClientState *)’ {aka ‘_Bool (*)(struct NetClientState *)’} > [-Werror=incompatible-pointer-types] > 305 | .can_receive = tulip_can_receive, > | ^~~~~~~~~~~~~~~~~ > > Maybe because of a stagged local change in your tree? (to confirm) Right, it's the conversion from int to bool done by Philippe :) I will fix the conflict after Qiang tests it. Thanks > > Thank you. > -- > Prasad J Pandit / Red Hat Product Security Team > 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
+-- On Mon, 23 Mar 2020, Jason Wang wrote --+ | Prasad, please fix this and post a new version. | | While at it, I prefer to squash patch 3 into patch 2 since patch 3 fixes the | issue introduced by patch 2. Sent patch v6. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
From: Prasad J Pandit <pjp@fedoraproject.org> Hello, * This series adds checks to avoid potential OOB access and infinite loop issues while processing rx/tx data. * Tulip tx descriptors are capped at 128 to avoid infinite loop in tulip_xmit_list_update(), wrt Tulip kernel driver -> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/dec/tulip/tulip.h#n319 * Update v3: add .can_receive routine -> https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06275.html * Update v4: flush queued packets once they are received -> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg05868.html * Update v5: fixed a typo in patch commit message -> https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg06209.html Thank you. -- Prasad J Pandit (3): net: tulip: check frame size and r/w data length net: tulip: add .can_receive routine net: tulip: flush queued packets post receive hw/net/tulip.c | 51 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 9 deletions(-) -- 2.25.1