diff mbox series

[03/10] sim/ppc: initialize a memory buffer in all cases

Message ID 0bd7df35b8c709a72d6f2a0900e60fa12727779e.1666192979.git.aburgess@redhat.com
State Committed
Headers
Series Building the sim/ tree with clang |

Commit Message

Andrew Burgess Oct. 19, 2022, 3:24 p.m. UTC 
  In the ppc simulator's do_fstat function, which provides the fstat
call for the simulator, if the fstat is going to fail then we current
write an uninitialized buffer into the inferior.

In theory, I think this is fine, we also write the error status into
the simulated target, so, given that the fstat has failed, the target
shouldn't be relying on the buffer contents.

However, writing an uninitialized buffer means we might leak simulator
private data into the simulated target, which is probably a bad
thing.  Plus it probably makes life easier if something consistent,
like all zeros, is written rather than random junk, that might look
like a successful call.

So, in this commit, I clear the stat buffer to zero before writing it
into the simulated target, if the fstat call is not run (due to a bad
file descriptor).

Another option would be to just not write the buffer into the
inferior (rather than zeroing it, and writing all zeros).  This would
solve the problem of copying simulator data into the target, but I
think the all zeros will make debugging things in the simulator
easier.
---
 sim/ppc/emul_netbsd.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Mike Frysinger Oct. 23, 2022, 1:50 p.m. UTC | #1 
On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
> --- a/sim/ppc/emul_netbsd.c
> +++ b/sim/ppc/emul_netbsd.c
> @@ -888,6 +888,8 @@ do_fstat(os_emul_data *emul,
>    status = fdbad (fd);
>    if (status == 0)
>      status = fstat(fd, &buf);
> +  else
> +    memset (&buf, 0, sizeof (buf));

i don't think this is perf critical, so simpler to do:
  struct statfs buf = {};
-mike
Andrew Burgess Oct. 24, 2022, 4:25 p.m. UTC | #2 
Mike Frysinger <vapier@gentoo.org> writes:

> On 19 Oct 2022 16:24, Andrew Burgess via Gdb-patches wrote:
>> --- a/sim/ppc/emul_netbsd.c
>> +++ b/sim/ppc/emul_netbsd.c
>> @@ -888,6 +888,8 @@ do_fstat(os_emul_data *emul,
>>    status = fdbad (fd);
>>    if (status == 0)
>>      status = fstat(fd, &buf);
>> +  else
>> +    memset (&buf, 0, sizeof (buf));
>
> i don't think this is perf critical, so simpler to do:
>   struct statfs buf = {};

Pushed with the fix you suggest.

Thanks,
Andrew
diff mbox series

Patch

diff --git a/sim/ppc/emul_netbsd.c b/sim/ppc/emul_netbsd.c
index 322b584a3f1..f5fa8499bde 100644
--- a/sim/ppc/emul_netbsd.c
+++ b/sim/ppc/emul_netbsd.c
@@ -888,6 +888,8 @@  do_fstat(os_emul_data *emul,
   status = fdbad (fd);
   if (status == 0)
     status = fstat(fd, &buf);
+  else
+    memset (&buf, 0, sizeof (buf));
   emul_write_status(processor, status, errno);
   write_stat(stat_buf_addr, buf, processor, cia);
 }