netdev - Re: [RFC net-next 3/7] net/ism: Use uuid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250120171820.GC6206@kernel.org>
Date: Mon, 20 Jan 2025 17:18:20 +0000
From: Simon Horman <horms@...nel.org>
To: Alexandra Winter <wintera@...ux.ibm.com>
Cc: Wenjia Zhang <wenjia@...ux.ibm.com>, Jan Karcher <jaka@...ux.ibm.com>,
	Gerd Bayer <gbayer@...ux.ibm.com>,
	Halil Pasic <pasic@...ux.ibm.com>,
	"D. Wythe" <alibuda@...ux.alibaba.com>,
	Tony Lu <tonylu@...ux.alibaba.com>,
	Wen Gu <guwen@...ux.alibaba.com>,
	Peter Oberparleiter <oberpar@...ux.ibm.com>,
	David Miller <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Julian Ruess <julianr@...ux.ibm.com>,
	Niklas Schnelle <schnelle@...ux.ibm.com>,
	Thorsten Winkler <twinkler@...ux.ibm.com>, netdev@...r.kernel.org,
	linux-s390@...r.kernel.org, Heiko Carstens <hca@...ux.ibm.com>,
	Vasily Gorbik <gor@...ux.ibm.com>,
	Alexander Gordeev <agordeev@...ux.ibm.com>,
	Christian Borntraeger <borntraeger@...ux.ibm.com>,
	Sven Schnelle <svens@...ux.ibm.com>
Subject: Re: [RFC net-next 3/7] net/ism: Use uuid_t for ISM GID

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@...ux.ibm.com>

...

> 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
>