All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: P J P <ppandit@redhat.com>
Cc: Stefan Weil <sw@weilnetz.de>, Jason Wang <jasowang@redhat.com>,
	Ruhr-University Bochum <bugs-syssec@rub.de>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Prasad J Pandit <pjp@fedoraproject.org>
Subject: Re: [PATCH] net: eepro100: validate various address values
Date: Thu, 18 Feb 2021 14:41:07 +0000	[thread overview]
Message-ID: <CAFEAcA_8sUX5nbg5+DR8Z6F3t1Y3o=tgZY56dFTKLgA7XVWOcw@mail.gmail.com> (raw)
In-Reply-To: <20210218140629.373646-1-ppandit@redhat.com>

On Thu, 18 Feb 2021 at 14:13, P J P <ppandit@redhat.com> wrote:
>
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While processing controller commands, eepro100 emulator gets
> command unit(CU) base address OR receive unit (RU) base address
> OR command block (CB) address from guest. If these values are not
> checked, it may lead to an infinite loop kind of issues. Add checks
> to avoid it.
>
> Reported-by: Ruhr-University Bochum <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/eepro100.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index 16e95ef9cc..afa1c9b2aa 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -843,7 +843,8 @@ static void action_command(EEPRO100State *s)
>          bool bit_i;
>          bool bit_nc;
>          uint16_t ok_status = STATUS_OK;
> -        s->cb_address = s->cu_base + s->cu_offset;
> +        s->cb_address = s->cu_base + s->cu_offset;  /* uint32_t overflow */
> +        assert (s->cb_address >= s->cu_base);

We get these values from the guest; you can't just assert() on them.
You need to do something else.

My reading of the 8255x data sheet is that there is nothing
in the hardware that forbids the guest from programming the
device such that the cu_base + cu_offset wraps around:
http://www.intel.com/content/dam/doc/manual/8255x-10-100-mbps-ethernet-controller-software-dev-manual.pdf
-- page 30 says that this is all doing 32-bit arithmetic
on addresses and doesn't say that there is any special case
handling by the device of overflow of that addition.

Your commit message isn't very clear about what the failure
case is here, but I think the fix has to be something
different from this.

thanks
-- PMM


  parent reply	other threads:[~2021-02-18 14:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-18 14:06 [PATCH] net: eepro100: validate various address values P J P
2021-02-18 14:18 ` no-reply
2021-02-18 14:41 ` Peter Maydell [this message]
2021-02-18 16:10   ` Stefan Weil
2021-02-19  1:54   ` Alexander Bulekov
2021-02-19  2:06     ` Li Qiang
2021-02-19  2:14       ` Alexander Bulekov
2021-02-19  4:43         ` Li Qiang
2021-02-20  3:05           ` Alexander Bulekov
2021-02-19  6:11     ` P J P
2021-02-19  8:08       ` Stefan Weil
2021-02-19  8:26         ` Stefan Weil
2021-02-19  9:26           ` P J P
2021-02-19  9:52             ` Stefan Weil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFEAcA_8sUX5nbg5+DR8Z6F3t1Y3o=tgZY56dFTKLgA7XVWOcw@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=bugs-syssec@rub.de \
    --cc=jasowang@redhat.com \
    --cc=pjp@fedoraproject.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.