On Wed, Jan 15, 2025 at 08:55:23PM +0100, Alexandra Winter wrote:
> SMC uses 64 Bit and 128 Bit Global Identifiers (GIDs)
> that need to be sent via the SMC protocol.
> When integers are used network endianness and host endianness
> need to be considered.
>
> Avoid this in the ISM layer by using uuid_t byte arrays.
> Follow on patches could do the same change for SMC, for now
> conversion helper functions are introduced.
>
> ISM-vPCI devices provide 64 Bit GIDs. Map them to ISM uuid_t GIDs
> like this:
> _________________________________________
> | 64 Bit ISM-vPCI GID | 00000000_00000000 |
> -----------------------------------------
> If interpreted as UUID, this would be interpreted as th UIID variant,
> that is reserved for NCS backward compatibility. So it will not collide
> with UUIDs that were generated according to the standard.
>
> Future ISM devices, shall use real UUIDs as 128 Bit GIDs.
>
> Note:
> - In this RFC patch smcd_gid is now moved back to smc.h,
> future patchset should avoid that.
> - ism_dmb and ism_event structs still contain 64 Bit rgid and info
> fields. A future patch could change them to uuid_t gids. This
> does not break anything, because ism_loopback does not use them.
>
> Signed-off-by: Alexandra Winter <wintera@xxxxxxxxxxxxx>
...
> diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h
> index 6763133dd8d0..d041e5a7c459 100644
> --- a/net/smc/smc_ism.h
> +++ b/net/smc/smc_ism.h
> @@ -12,6 +12,7 @@
> #include <linux/uio.h>
> #include <linux/types.h>
> #include <linux/mutex.h>
> +#include <linux/ism.h>
>
> #include "smc.h"
>
> @@ -94,4 +95,24 @@ static inline bool smc_ism_is_loopback(struct smcd_dev *smcd)
> return (smcd->ops->get_chid(smcd) == 0xFFFF);
> }
>
> +static inline void copy_to_smcdgid(struct smcd_gid *sgid, uuid_t *igid)
> +{
> + __be64 temp;
> +
> + memcpy(&temp, igid, sizeof(sgid->gid));
> + sgid->gid = ntohll(temp);
> + memcpy(&temp, igid + sizeof(sgid->gid), sizeof(sgid->gid_ext));
Hi Alexandra,
The stride of the pointer arithmetic is the width of igid
so this write will be at an offset of:
sizeof(igid) + sizeof(sgid->gid) = 128 bytes
Which is beyond the end of *igid.
I think the desired operation is to write at an offset of 8 bytes, so
perhaps this is a way to achieve that, as the bi field is a
16 byte array of u8:
memcpy(&temp, igid->b + sizeof(sgid->gid), sizeof(sgid->gid_ext));
Flagged by W=1 builds with gcc-14 and clang-19, and by Smatch.
> + sgid->gid_ext = ntohll(temp);
> +}
> +
> +static inline void copy_to_ismgid(uuid_t *igid, struct smcd_gid *sgid)
> +{
> + __be64 temp;
> +
> + temp = htonll(sgid->gid);
> + memcpy(igid, &temp, sizeof(sgid->gid));
> + temp = htonll(sgid->gid_ext);
> + memcpy(igid + sizeof(sgid->gid), &temp, sizeof(sgid->gid_ext));
I believe there is a similar problem here too.
> +}
> +
> #endif
> --
> 2.45.2
>
