From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>,
qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: qemu-stable@nongnu.org
Subject: Re: [PATCH] hw/net/smc91c111: Don't allow data register access to overrun buffer
Date: Tue, 11 Mar 2025 10:01:13 +0100 [thread overview]
Message-ID: <f325c69e-6a30-4d16-bce1-8f7b4de72de1@linaro.org> (raw)
In-Reply-To: <20250228191652.1957208-1-peter.maydell@linaro.org>
On 28/2/25 20:16, Peter Maydell wrote:
> For accesses to the 91c111 data register, the address within the
> packet's data frame is determined by a combination of the pointer
> register and the offset used to access the data register, so that you
> can access data at effectively wider than byte width. The pointer
> register's pointer field is 11 bits wide, which is exactly the size
> to index a 2048-byte data frame.
>
> We weren't quite getting the logic right for ensuring that we end up
> with a pointer value to use in the s->data[][] array that isn't out
> of bounds:
>
> * we correctly mask when getting the initial pointer value
> * for the "autoincrement the pointer register" case, we
> correctly mask after adding 1 so that the pointer register
> wraps back around at the 2048 byte mark
> * but for the non-autoincrement case where we have to add the
> low 2 bits of the data register offset, we don't account
> for the possibility that the pointer register is 0x7ff
> and the addition should wrap
>
> Fix this bug by factoring out the "get the p value to use as an array
> index" into a function, making it use FIELD macro names rather than
> hard-coded constants, and having a utility function that does "add a
> value and wrap it" that we can use both for the "autoincrement" and
> "add the offset bits" codepaths.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2758
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/net/smc91c111.c | 65 +++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 53 insertions(+), 12 deletions(-)
Patch queued via hw-misc (except if another tree is preferred), thanks!
prev parent reply other threads:[~2025-03-11 9:02 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-28 19:16 [PATCH] hw/net/smc91c111: Don't allow data register access to overrun buffer Peter Maydell
2025-03-10 1:32 ` Philippe Mathieu-Daudé
2025-03-11 9:01 ` Philippe Mathieu-Daudé [this message]
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=f325c69e-6a30-4d16-bce1-8f7b4de72de1@linaro.org \
--to=philmd@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).