Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-26 Thread BALATON Zoltan

On Fri, 26 Apr 2024, Philippe Mathieu-Daudé wrote:

On 26/4/24 14:37, Akihiko Odaki wrote:

On 2024/04/24 21:32, Thomas Huth wrote:

On 24/04/2024 12.41, Prasad Pandit wrote:
On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé 
wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update 
it

when delivering a packet to a device.
In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different fix 
patch.

   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
-> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the 
latter is a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

  https://gitlab.com/qemu-project/qemu/-/issues/556
  i.e.:
  https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3


I agree. Commit c40ca2301c7603524eaddb5308a3 should be what fixed 
CVE-2021-20255, not this patch.


Thank you all for clarifying!


$ git log -p c40ca2301c7603524eaddb5308a3 --
fatal: bad revision 'c40ca2301c7603524eaddb5308a3'

It seems to actually be commit a2e1753b8054344f32cf94f31c6399a58794a380

Regards,
BALATON Zoltan

Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-26 Thread Philippe Mathieu-Daudé

On 26/4/24 14:37, Akihiko Odaki wrote:

On 2024/04/24 21:32, Thomas Huth wrote:

On 24/04/2024 12.41, Prasad Pandit wrote:
On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe 
Mathieu-Daudé wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:
Recently MemReentrancyGuard was added to DeviceState to record that 
the
device is engaging in I/O. The network device backend needs to 
update it

when delivering a packet to a device.
In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different 
fix patch.

   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
-> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the 
latter is a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

  https://gitlab.com/qemu-project/qemu/-/issues/556
  i.e.:
  https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3


I agree. Commit c40ca2301c7603524eaddb5308a3 should be what fixed 
CVE-2021-20255, not this patch.


Thank you all for clarifying!




Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-26 Thread Akihiko Odaki

On 2024/04/24 21:32, Thomas Huth wrote:

On 24/04/2024 12.41, Prasad Pandit wrote:
On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe 
Mathieu-Daudé wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to 
update it

when delivering a packet to a device.
In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different 
fix patch.

   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
   
-> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html

   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the 
latter is a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

  https://gitlab.com/qemu-project/qemu/-/issues/556
  i.e.:
  https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3


I agree. Commit c40ca2301c7603524eaddb5308a3 should be what fixed 
CVE-2021-20255, not this patch.


Regards,
Akihiko Odaki



Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Thomas Huth

On 24/04/2024 12.41, Prasad Pandit wrote:

On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.
  
In preparation for such a change, add MemReentrancyGuard * as a

parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different fix patch.
   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
   -> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the latter is 
a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

 https://gitlab.com/qemu-project/qemu/-/issues/556
 i.e.:
 https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3

 HTH,
  Thomas





Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Prasad Pandit
On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé wrote:
>On 1/6/23 05:18, Akihiko Odaki wrote:
>> Recently MemReentrancyGuard was added to DeviceState to record that the
>> device is engaging in I/O. The network device backend needs to update it
>> when delivering a packet to a device.
>> 
>> In preparation for such a change, add MemReentrancyGuard * as a
>> parameter of qemu_new_nic().
>
>An user on IRC asked if this patch is related/fixing CVE-2021-20255,
>any clue?

* CVE-2021-20255 bug: infinite recursion is pointing at a different fix patch.
  -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
  -> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
  Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the latter is 
a use-after-free one.


Thank you.
---
  -Prasad



Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Philippe Mathieu-Daudé

Hi,

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.

In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


Signed-off-by: Akihiko Odaki 
---
  include/net/net.h | 1 +
  hw/net/allwinner-sun8i-emac.c | 3 ++-
  hw/net/allwinner_emac.c   | 3 ++-
  hw/net/cadence_gem.c  | 3 ++-
  hw/net/dp8393x.c  | 3 ++-
  hw/net/e1000.c| 3 ++-
  hw/net/e1000e.c   | 2 +-
  hw/net/eepro100.c | 4 +++-
  hw/net/etraxfs_eth.c  | 3 ++-
  hw/net/fsl_etsec/etsec.c  | 3 ++-
  hw/net/ftgmac100.c| 3 ++-
  hw/net/i82596.c   | 2 +-
  hw/net/igb.c  | 2 +-
  hw/net/imx_fec.c  | 2 +-
  hw/net/lan9118.c  | 3 ++-
  hw/net/mcf_fec.c  | 3 ++-
  hw/net/mipsnet.c  | 3 ++-
  hw/net/msf2-emac.c| 3 ++-
  hw/net/mv88w8618_eth.c| 3 ++-
  hw/net/ne2000-isa.c   | 3 ++-
  hw/net/ne2000-pci.c   | 3 ++-
  hw/net/npcm7xx_emc.c  | 3 ++-
  hw/net/opencores_eth.c| 3 ++-
  hw/net/pcnet.c| 3 ++-
  hw/net/rocker/rocker_fp.c | 4 ++--
  hw/net/rtl8139.c  | 3 ++-
  hw/net/smc91c111.c| 3 ++-
  hw/net/spapr_llan.c   | 3 ++-
  hw/net/stellaris_enet.c   | 3 ++-
  hw/net/sungem.c   | 2 +-
  hw/net/sunhme.c   | 3 ++-
  hw/net/tulip.c| 3 ++-
  hw/net/virtio-net.c   | 6 --
  hw/net/vmxnet3.c  | 2 +-
  hw/net/xen_nic.c  | 4 ++--
  hw/net/xgmac.c| 3 ++-
  hw/net/xilinx_axienet.c   | 3 ++-
  hw/net/xilinx_ethlite.c   | 3 ++-
  hw/usb/dev-network.c  | 3 ++-
  net/net.c | 1 +
  40 files changed, 75 insertions(+), 41 deletions(-)





Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2023-06-05 Thread Akihiko Odaki

On 2023/06/05 17:06, Alexander Bulekov wrote:

On 230601 1218, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.

In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().

Signed-off-by: Akihiko Odaki 



Reviewed-by: Alexander Bulekov 

One minor comment below.


---
  include/net/net.h | 1 +
  hw/net/allwinner-sun8i-emac.c | 3 ++-
  hw/net/allwinner_emac.c   | 3 ++-
  hw/net/cadence_gem.c  | 3 ++-
  hw/net/dp8393x.c  | 3 ++-
  hw/net/e1000.c| 3 ++-
  hw/net/e1000e.c   | 2 +-
  hw/net/eepro100.c | 4 +++-
  hw/net/etraxfs_eth.c  | 3 ++-
  hw/net/fsl_etsec/etsec.c  | 3 ++-
  hw/net/ftgmac100.c| 3 ++-
  hw/net/i82596.c   | 2 +-
  hw/net/igb.c  | 2 +-
  hw/net/imx_fec.c  | 2 +-
  hw/net/lan9118.c  | 3 ++-
  hw/net/mcf_fec.c  | 3 ++-
  hw/net/mipsnet.c  | 3 ++-
  hw/net/msf2-emac.c| 3 ++-
  hw/net/mv88w8618_eth.c| 3 ++-
  hw/net/ne2000-isa.c   | 3 ++-
  hw/net/ne2000-pci.c   | 3 ++-
  hw/net/npcm7xx_emc.c  | 3 ++-
  hw/net/opencores_eth.c| 3 ++-
  hw/net/pcnet.c| 3 ++-
  hw/net/rocker/rocker_fp.c | 4 ++--
  hw/net/rtl8139.c  | 3 ++-
  hw/net/smc91c111.c| 3 ++-
  hw/net/spapr_llan.c   | 3 ++-
  hw/net/stellaris_enet.c   | 3 ++-
  hw/net/sungem.c   | 2 +-
  hw/net/sunhme.c   | 3 ++-
  hw/net/tulip.c| 3 ++-
  hw/net/virtio-net.c   | 6 --
  hw/net/vmxnet3.c  | 2 +-
  hw/net/xen_nic.c  | 4 ++--
  hw/net/xgmac.c| 3 ++-
  hw/net/xilinx_axienet.c   | 3 ++-
  hw/net/xilinx_ethlite.c   | 3 ++-
  hw/usb/dev-network.c  | 3 ++-
  net/net.c | 1 +
  40 files changed, 75 insertions(+), 41 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 1448d00afb..a7d8deaccb 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -157,6 +157,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
 NICConf *conf,
 const char *model,
 const char *name,
+   MemReentrancyGuard *reentrancy_guard,
 void *opaque);


Does it make sense to roll *reentrancy_guard into NICConf here?


Probably no. All members of NICConf are device properties the user 
configure, but reentrancy_guard is an internal state. Similarly, the 
opaque parameter is an internal state and not included in NICConf.




Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2023-06-05 Thread Alexander Bulekov
On 230601 1218, Akihiko Odaki wrote:
> Recently MemReentrancyGuard was added to DeviceState to record that the
> device is engaging in I/O. The network device backend needs to update it
> when delivering a packet to a device.
> 
> In preparation for such a change, add MemReentrancyGuard * as a
> parameter of qemu_new_nic().
> 
> Signed-off-by: Akihiko Odaki 


Reviewed-by: Alexander Bulekov 

One minor comment below.

> ---
>  include/net/net.h | 1 +
>  hw/net/allwinner-sun8i-emac.c | 3 ++-
>  hw/net/allwinner_emac.c   | 3 ++-
>  hw/net/cadence_gem.c  | 3 ++-
>  hw/net/dp8393x.c  | 3 ++-
>  hw/net/e1000.c| 3 ++-
>  hw/net/e1000e.c   | 2 +-
>  hw/net/eepro100.c | 4 +++-
>  hw/net/etraxfs_eth.c  | 3 ++-
>  hw/net/fsl_etsec/etsec.c  | 3 ++-
>  hw/net/ftgmac100.c| 3 ++-
>  hw/net/i82596.c   | 2 +-
>  hw/net/igb.c  | 2 +-
>  hw/net/imx_fec.c  | 2 +-
>  hw/net/lan9118.c  | 3 ++-
>  hw/net/mcf_fec.c  | 3 ++-
>  hw/net/mipsnet.c  | 3 ++-
>  hw/net/msf2-emac.c| 3 ++-
>  hw/net/mv88w8618_eth.c| 3 ++-
>  hw/net/ne2000-isa.c   | 3 ++-
>  hw/net/ne2000-pci.c   | 3 ++-
>  hw/net/npcm7xx_emc.c  | 3 ++-
>  hw/net/opencores_eth.c| 3 ++-
>  hw/net/pcnet.c| 3 ++-
>  hw/net/rocker/rocker_fp.c | 4 ++--
>  hw/net/rtl8139.c  | 3 ++-
>  hw/net/smc91c111.c| 3 ++-
>  hw/net/spapr_llan.c   | 3 ++-
>  hw/net/stellaris_enet.c   | 3 ++-
>  hw/net/sungem.c   | 2 +-
>  hw/net/sunhme.c   | 3 ++-
>  hw/net/tulip.c| 3 ++-
>  hw/net/virtio-net.c   | 6 --
>  hw/net/vmxnet3.c  | 2 +-
>  hw/net/xen_nic.c  | 4 ++--
>  hw/net/xgmac.c| 3 ++-
>  hw/net/xilinx_axienet.c   | 3 ++-
>  hw/net/xilinx_ethlite.c   | 3 ++-
>  hw/usb/dev-network.c  | 3 ++-
>  net/net.c | 1 +
>  40 files changed, 75 insertions(+), 41 deletions(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 1448d00afb..a7d8deaccb 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -157,6 +157,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
> NICConf *conf,
> const char *model,
> const char *name,
> +   MemReentrancyGuard *reentrancy_guard,
> void *opaque);

Does it make sense to roll *reentrancy_guard into NICConf here?



[PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2023-05-31 Thread Akihiko Odaki
Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.

In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().

Signed-off-by: Akihiko Odaki 
---
 include/net/net.h | 1 +
 hw/net/allwinner-sun8i-emac.c | 3 ++-
 hw/net/allwinner_emac.c   | 3 ++-
 hw/net/cadence_gem.c  | 3 ++-
 hw/net/dp8393x.c  | 3 ++-
 hw/net/e1000.c| 3 ++-
 hw/net/e1000e.c   | 2 +-
 hw/net/eepro100.c | 4 +++-
 hw/net/etraxfs_eth.c  | 3 ++-
 hw/net/fsl_etsec/etsec.c  | 3 ++-
 hw/net/ftgmac100.c| 3 ++-
 hw/net/i82596.c   | 2 +-
 hw/net/igb.c  | 2 +-
 hw/net/imx_fec.c  | 2 +-
 hw/net/lan9118.c  | 3 ++-
 hw/net/mcf_fec.c  | 3 ++-
 hw/net/mipsnet.c  | 3 ++-
 hw/net/msf2-emac.c| 3 ++-
 hw/net/mv88w8618_eth.c| 3 ++-
 hw/net/ne2000-isa.c   | 3 ++-
 hw/net/ne2000-pci.c   | 3 ++-
 hw/net/npcm7xx_emc.c  | 3 ++-
 hw/net/opencores_eth.c| 3 ++-
 hw/net/pcnet.c| 3 ++-
 hw/net/rocker/rocker_fp.c | 4 ++--
 hw/net/rtl8139.c  | 3 ++-
 hw/net/smc91c111.c| 3 ++-
 hw/net/spapr_llan.c   | 3 ++-
 hw/net/stellaris_enet.c   | 3 ++-
 hw/net/sungem.c   | 2 +-
 hw/net/sunhme.c   | 3 ++-
 hw/net/tulip.c| 3 ++-
 hw/net/virtio-net.c   | 6 --
 hw/net/vmxnet3.c  | 2 +-
 hw/net/xen_nic.c  | 4 ++--
 hw/net/xgmac.c| 3 ++-
 hw/net/xilinx_axienet.c   | 3 ++-
 hw/net/xilinx_ethlite.c   | 3 ++-
 hw/usb/dev-network.c  | 3 ++-
 net/net.c | 1 +
 40 files changed, 75 insertions(+), 41 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 1448d00afb..a7d8deaccb 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -157,6 +157,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
NICConf *conf,
const char *model,
const char *name,
+   MemReentrancyGuard *reentrancy_guard,
void *opaque);
 void qemu_del_nic(NICState *nic);
 NetClientState *qemu_get_subqueue(NICState *nic, int queue_index);
diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
index fac4405f45..cc350d40e5 100644
--- a/hw/net/allwinner-sun8i-emac.c
+++ b/hw/net/allwinner-sun8i-emac.c
@@ -824,7 +824,8 @@ static void allwinner_sun8i_emac_realize(DeviceState *dev, 
Error **errp)
 
 qemu_macaddr_default_if_unset(>conf.macaddr);
 s->nic = qemu_new_nic(_allwinner_sun8i_emac_info, >conf,
-   object_get_typename(OBJECT(dev)), dev->id, s);
+  object_get_typename(OBJECT(dev)), dev->id,
+  >mem_reentrancy_guard, s);
 qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 }
 
diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
index 372e5b66da..e10965de14 100644
--- a/hw/net/allwinner_emac.c
+++ b/hw/net/allwinner_emac.c
@@ -453,7 +453,8 @@ static void aw_emac_realize(DeviceState *dev, Error **errp)
 
 qemu_macaddr_default_if_unset(>conf.macaddr);
 s->nic = qemu_new_nic(_aw_emac_info, >conf,
-  object_get_typename(OBJECT(dev)), dev->id, s);
+  object_get_typename(OBJECT(dev)), dev->id,
+  >mem_reentrancy_guard, s);
 qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
 fifo8_create(>rx_fifo, RX_FIFO_SIZE);
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 42ea2411a2..a7bce1c120 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1633,7 +1633,8 @@ static void gem_realize(DeviceState *dev, Error **errp)
 qemu_macaddr_default_if_unset(>conf.macaddr);
 
 s->nic = qemu_new_nic(_gem_info, >conf,
-  object_get_typename(OBJECT(dev)), dev->id, s);
+  object_get_typename(OBJECT(dev)), dev->id,
+  >mem_reentrancy_guard, s);
 
 if (s->jumbo_max_len > MAX_FRAME_SIZE) {
 error_setg(errp, "jumbo-max-len is greater than %d",
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 45b954e46c..abfcc6f69f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -943,7 +943,8 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
   "dp8393x-regs", SONIC_REG_COUNT << s->it_shift);
 
 s->nic = qemu_new_nic(_dp83932_info, >conf,
-  object_get_typename(OBJECT(dev)), dev->id, s);
+  object_get_typename(OBJECT(dev)), dev->id,
+  >mem_reentrancy_guard, s);
 qemu_format_nic_info_str(qemu_get_queue(s->nic),