All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Anderson <sean.anderson@linux.dev>
To: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	"Luc Michel" <lmichel@kalray.eu>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH 3/3] semihosting: Check for overflow in FLEN on 32-bit systems
Date: Mon, 20 Oct 2025 10:21:29 -0400	[thread overview]
Message-ID: <56d70072-67ee-471a-9b9a-c3257886d668@linux.dev> (raw)
In-Reply-To: <4d1a679a-f1c2-487b-bddb-eaf7dd56fd0e@canonical.com>

On 10/18/25 03:21, Heinrich Schuchardt wrote:
> On 10/17/25 23:35, Sean Anderson wrote:
>> When semihosting 32-bit systems, the return value of FLEN will be stored
>> in a 32-bit integer. To prevent wraparound, return -1 and set EOVERFLOW.
>> This matches the behavior of stat(2). Static files don't need to be
>> checked, since are always small.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>>
>>   semihosting/arm-compat-semi.c | 17 ++++++++++++++---
>>   1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
>> index c5a07cb947..57453ca6be 100644
>> --- a/semihosting/arm-compat-semi.c
>> +++ b/semihosting/arm-compat-semi.c
>> @@ -305,8 +305,19 @@ static uint64_t common_semi_flen_buf(CPUState *cs)
>>       return sp - 64;
>>   }
>>   +static void common_semi_flen_cb(CPUState *cs, uint64_t ret, int err)
>> +{
>> +    CPUArchState *env = cpu_env(cs);
>> +
>> +    if (!err && !is_64bit_semihosting(env) && ret > INT32_MAX) {
> 
> 
> The issue with the current implementation is that files with file sizes over 4 GiB will be reported as file size < 4 -GiB on 32bit systems. Thanks for addressing this.
> 
> But unfortunately with your change you are additionally dropping support for file sizes 2 GiB to 4 GiB on 32bit devices. This should be avoided.
> 
> The semihosting specification specifies that the value returned in r0 should be -1 if an error occurs. So on 32 bit systems 0xffffffff should be returned.
> 
> As file sizes cannot be negative there is not reason to assume that the value in r0 has to be interpreted by semihosting clients as signed.
> 
> Please, change your commit to check against 0xffffffff.
> 
> It might make sense to contact ARM to make their specification clearer.

stat(2) will return -1/EOVERFLOW on 32-bit hosts for files over 2 GiB. I believe we should be consistent.

--Sean


  reply	other threads:[~2025-10-20 15:01 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-17 21:35 [PATCH 0/3] semihosting: Fix a few semihosting bugs Sean Anderson
2025-10-17 21:35 ` [PATCH 1/3] gdbstub: Fix %s formatting Sean Anderson
2025-10-18  0:07   ` Richard Henderson
2025-10-20 15:05   ` Alex Bennée
2025-10-17 21:35 ` [PATCH 2/3] semihosting: Fix GDB File-I/O FLEN Sean Anderson
2025-10-20 16:25   ` Alex Bennée
2025-10-17 21:35 ` [PATCH 3/3] semihosting: Check for overflow in FLEN on 32-bit systems Sean Anderson
2025-10-18  7:21   ` Heinrich Schuchardt
2025-10-20 14:21     ` Sean Anderson [this message]
2025-10-20 15:33       ` Heinrich Schuchardt
2025-10-20 15:39         ` Sean Anderson
2025-10-20 16:33       ` Peter Maydell
2025-10-20 19:31         ` Sean Anderson
2025-10-20 15:03 ` [PATCH 0/3] semihosting: Fix a few semihosting bugs Alex Bennée
2025-10-20 15:06   ` Sean Anderson
2025-10-27 10:54 ` Alex Bennée
2025-10-31 10:31 ` Michael Tokarev
2025-10-31 11:44   ` Alex Bennée

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=56d70072-67ee-471a-9b9a-c3257886d668@linux.dev \
    --to=sean.anderson@linux.dev \
    --cc=alex.bennee@linaro.org \
    --cc=heinrich.schuchardt@canonical.com \
    --cc=lmichel@kalray.eu \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.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 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.