Re: [PATCH v5 3/3] hw/nvme: add nvme management interface model

[Andrew Jeffery]

Hi Klaus,

On Tue, 2023-09-05 at 10:38 +0200, Klaus Jensen wrote:
> > 
> > +static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest
> > *request)
> > +{
> > +    uint32_t dw0 = le32_to_cpu(request->dw0);
> > +    uint8_t identifier = FIELD_EX32(dw0,
> > NMI_CMD_CONFIGURATION_GET_DW0,
> > +                                    IDENTIFIER);
> > +    const uint8_t *buf;
> > +
> > +    static const uint8_t smbus_freq[4] = {
> > +        0x00,               /* success */
> > +        0x01, 0x00, 0x00,   /* 100 kHz */
> > +    };
> > +
> > +    static const uint8_t mtu[4] = {
> > +        0x00,       /* success */
> > +        0x40, 0x00, /* 64 */
> > +        0x00,       /* reserved */
> > +    };
> > +
> > +    trace_nmi_handle_mi_config_get(identifier);
> > +
> > +    switch (identifier) {
> > +    case NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ:
> > +        buf = smbus_freq;
> > +        break;
> > +
> > +    case NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT:
> > +        buf = mtu;
> > +        break;
> > +
> > +    default:
> > +        nmi_set_parameter_error(nmi, 0x0, offsetof(NMIRequest,
> > dw0));
> > +        return;
> > +    }
> > +
> > +    nmi_scratch_append(nmi, buf, sizeof(buf));
> > +}

When I tried to build this patch I got:

```
In file included from /usr/include/string.h:535,
                 from /home/andrew/src/qemu.org/qemu/include/qemu/osdep.h:112,
                 from ../hw/nvme/nmi-i2c.c:12:
In function ‘memcpy’,
    inlined from ‘nmi_scratch_append’ at ../hw/nvme/nmi-i2c.c:80:5,
    inlined from ‘nmi_handle_mi_config_get’ at ../hw/nvme/nmi-i2c.c:246:5,
    inlined from ‘nmi_handle_mi’ at ../hw/nvme/nmi-i2c.c:266:9,
    inlined from ‘nmi_handle’ at ../hw/nvme/nmi-i2c.c:313:9:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: error: 
‘__builtin_memcpy’ forming offset [4, 7] is out of the bounds [0, 4] 
[-Werror=array-bounds=]
   29 |   return __builtin___memcpy_chk (__dest, __src, __len,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   30 |                                  __glibc_objsize0 (__dest));
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
```

It wasn't clear initially from the error that the source of the problem
was the size associated with the source buffer, especially as there is
some pointer arithmetic being done to derive `__dest`.

Anyway, what we're trying to express is that the size to copy from buf
is the size of the array pointed to by buf. However, buf is declared as
a pointer to uint8_t, which loses the length information. To fix that I
think we need:

- const uint8_t *buf;
+ const uint8_t (*buf)[4];

and then:

- nmi_scratch_append(nmi, buf, sizeof(buf));
+ nmi_scratch_append(nmi, buf, sizeof(*buf));

Andrew