qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] eepro100: Don't allow guests to fail assertions


From: Stefan Weil
Subject: Re: [Qemu-devel] [PATCH] eepro100: Don't allow guests to fail assertions
Date: Wed, 23 Sep 2009 20:02:59 +0200
User-agent: Mozilla-Thunderbird 2.0.0.22 (X11/20090707)

Kevin Wolf schrieb:
> The idea of using assert() for input validation is rather questionable.
> Let's remove it from eepro100, so that guests need to find more
> interesting
> ways if they want to crash qemu.
>
> This patch replaces asserts that are directly dependent on
> guest-accessible
> data by other means of error handling.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> hw/eepro100.c | 46 +++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index d3b7c3d..9099da3 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -69,7 +69,7 @@
>
> #define TRACE(flag, command) ((flag) ? (command) : (void)0)
>
> -#define missing(text) assert(!"feature is missing in this emulation:
> " text)
> +#define missing(text) fprintf(stderr, "eepro100: feature is missing
> in this emulation: " text "\n")
>
> #define MAX_ETH_FRAME_SIZE 1514
>
> @@ -662,6 +662,7 @@ static void eepro100_cu_command(EEPRO100State * s,
> uint8_t val)
> bool bit_s = ((command & 0x4000) != 0);
> bool bit_i = ((command & 0x2000) != 0);
> bool bit_nc = ((command & 0x0010) != 0);
> + bool success = true;
> //~ bool bit_sf = ((command & 0x0008) != 0);
> uint16_t cmd = command & 0x0007;
> s->cu_offset = le32_to_cpu(tx.link);
> @@ -688,9 +689,17 @@ static void eepro100_cu_command(EEPRO100State *
> s, uint8_t val)
> logout
> ("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count
> %u\n",
> tbd_array, tcb_bytes, tx.tbd_count);
> - assert(!bit_nc);
> +
> + if (bit_nc) {
> + missing("CmdTx: NC = 0");
> + success = false;
> + break;
> + }
> //~ assert(!bit_sf);
> - assert(tcb_bytes <= 2600);
> + if (tcb_bytes > 2600) {
> + logout("TCB byte count too large, using 2600\n");
> + tcb_bytes = 2600;
> + }
> /* Next assertion fails for local configuration. */
> //~ assert((tcb_bytes > 0) || (tbd_array != 0xffffffff));
> if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) {
> @@ -722,7 +731,6 @@ static void eepro100_cu_command(EEPRO100State * s,
> uint8_t val)
> uint8_t tbd_count = 0;
> if (device_supports_eTxCB(s) && !(s->configuration[6] & BIT(4))) {
> /* Extended Flexible TCB. */
> - assert(tcb_bytes == 0);
> for (; tbd_count < 2; tbd_count++) {
> uint32_t tx_buffer_address = ldl_phys(tbd_address);
> uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
> @@ -771,9 +779,11 @@ static void eepro100_cu_command(EEPRO100State *
> s, uint8_t val)
> break;
> default:
> missing("undefined command");
> + success = false;
> + break;
> }
> - /* Write new status (success). */
> - stw_phys(cb_address, status | 0x8000 | 0x2000);
> + /* Write new status. */
> + stw_phys(cb_address, status | 0x8000 | (success ? 0x2000 : 0));
> if (bit_i) {
> /* CU completed action. */
> eepro100_cx_interrupt(s);
> @@ -1453,7 +1463,10 @@ static ssize_t nic_receive(VLANClientState *vc,
> const uint8_t * buf, size_t size
> { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> /* TODO: check multiple IA bit. */
> - assert(!(s->configuration[20] & BIT(6)));
> + if (s->configuration[20] & BIT(6)) {
> + missing("Multiple IA bit");
> + return -1;
> + }
>
> if (s->configuration[8] & 0x80) {
> /* CSMA is disabled. */
> @@ -1482,7 +1495,9 @@ static ssize_t nic_receive(VLANClientState *vc,
> const uint8_t * buf, size_t size
> /* Multicast frame. */
> logout("%p received multicast, len=%d\n", s, size);
> /* TODO: check multicast all bit. */
> - assert(!(s->configuration[21] & BIT(3)));
> + if (s->configuration[21] & BIT(3)) {
> + missing("Multicast All bit");
> + }
> int mcast_idx = compute_mcast_idx(buf);
> if (!(s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7)))) {
> return size;
> @@ -1512,7 +1527,12 @@ static ssize_t nic_receive(VLANClientState *vc,
> const uint8_t * buf, size_t size
> offsetof(eepro100_rx_t, packet));
> uint16_t rfd_command = le16_to_cpu(rx.command);
> uint16_t rfd_size = le16_to_cpu(rx.size);
> - assert(size <= rfd_size);
> +
> + if (size > rfd_size) {
> + logout("Receive buffer (%" PRId16 " bytes) too small for data "
> + "(%zu bytes); data truncated\n", rfd_size, size);
> + size = rfd_size;
> + }
> if (size < 64) {
> rfd_status |= 0x0080;
> }
> @@ -1524,7 +1544,10 @@ static ssize_t nic_receive(VLANClientState *vc,
> const uint8_t * buf, size_t size
> /* Early receive interrupt not supported. */
> //~ eepro100_er_interrupt(s);
> /* Receive CRC Transfer not supported. */
> - assert(!(s->configuration[18] & 4));
> + if (s->configuration[18] & 4) {
> + missing("Receive CRC Transfer");
> + return -1;
> + }
> /* TODO: check stripping enable bit. */
> //~ assert(!(s->configuration[17] & 1));
> cpu_physical_memory_write(s->ru_base + s->ru_offset +
> @@ -1534,7 +1557,8 @@ static ssize_t nic_receive(VLANClientState *vc,
> const uint8_t * buf, size_t size
> s->ru_offset = le32_to_cpu(rx.link);
> if (rfd_command & 0x8000) {
> /* EL bit is set, so this was the last frame. */
> - assert(0);
> + logout("receive: Running out of frames\n");
> + set_ru_state(s, ru_suspended);
> }
> if (rfd_command & 0x4000) {
> /* S bit is set. */


There are about 10 pending patches for eepro100.c, so the code at
savannah is
not a good base for new patches of eepro100.c. And there are even more
patches
in my queue to be sent as soon as the first 10 are integrated.

git://repo.or.cz/qemu/ar7.git has my latest version of eepro100.c.
Could you please send a patch based on that version (or just wait
until the savannah version is up-to-date)?

Regards
Stefan







reply via email to

[Prev in Thread] Current Thread [Next in Thread]