All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Ujwal Kundur <ujwal.kundur@gmail.com>
Cc: allison.henderson@oracle.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org, rds-devel@oss.oracle.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net] rds: Fix endian annotations across various assignments
Date: Sun, 10 Aug 2025 18:47:05 +0100	[thread overview]
Message-ID: <20250810174705.GK222315@ZenIV> (raw)
In-Reply-To: <20250810171155.3263-1-ujwal.kundur@gmail.com>

On Sun, Aug 10, 2025 at 10:41:55PM +0530, Ujwal Kundur wrote:

> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index 086a13170e09..9cd5905d916a 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c
> @@ -242,7 +242,7 @@ static __poll_t rds_poll(struct file *file, struct socket *sock,
>  	if (rs->rs_snd_bytes < rds_sk_sndbuf(rs))
>  		mask |= (EPOLLOUT | EPOLLWRNORM);
>  	if (sk->sk_err || !skb_queue_empty(&sk->sk_error_queue))
> -		mask |= POLLERR;
> +		mask |= (__force __poll_t)POLLERR;

EPOLLERR.

>  	read_unlock_irqrestore(&rs->rs_recv_lock, flags);
>  
>  	/* clear state any time we wake a seen-congested socket */
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index d62f486ab29f..0047b76c3c0b 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -62,13 +62,13 @@ static struct hlist_head *rds_conn_bucket(const struct in6_addr *laddr,
>  	net_get_random_once(&rds_hash_secret, sizeof(rds_hash_secret));
>  	net_get_random_once(&rds6_hash_secret, sizeof(rds6_hash_secret));
>  
> -	lhash = (__force u32)laddr->s6_addr32[3];
> +	lhash = (__force __u32)laddr->s6_addr32[3];
>  #if IS_ENABLED(CONFIG_IPV6)
>  	fhash = __ipv6_addr_jhash(faddr, rds6_hash_secret);
>  #else
> -	fhash = (__force u32)faddr->s6_addr32[3];
> +	fhash = (__force __u32)(faddr->s6_addr32[3]);
>  #endif
> -	hash = __inet_ehashfn(lhash, 0, fhash, 0, rds_hash_secret);
> +	hash = __inet_ehashfn((__force __be32)lhash, 0, (__force __be32)fhash, 0, rds_hash_secret);

what the...  You have lhash and fhash set to __be32 values, then
feed them to function that expects __be32 argument.  Just turn
these two variables into __be32 and lose the pointless casts.

> diff --git a/net/rds/rds.h b/net/rds/rds.h
> index dc360252c515..c2fb47e1fe07 100644
> --- a/net/rds/rds.h
> +++ b/net/rds/rds.h
> @@ -93,7 +93,7 @@ enum {
>  
>  /* Max number of multipaths per RDS connection. Must be a power of 2 */
>  #define	RDS_MPATH_WORKERS	8
> -#define	RDS_MPATH_HASH(rs, n) (jhash_1word((rs)->rs_bound_port, \
> +#define	RDS_MPATH_HASH(rs, n) (jhash_1word((__force __u16)(rs)->rs_bound_port, \
>  			       (rs)->rs_hash_initval) & ((n) - 1))

Reasonable.

>  #define IS_CANONICAL(laddr, faddr) (htonl(laddr) < htonl(faddr))
> diff --git a/net/rds/recv.c b/net/rds/recv.c
> index 5627f80013f8..7fc7a3850a7b 100644
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -216,10 +216,10 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
>  		switch (type) {
>  		case RDS_EXTHDR_NPATHS:
>  			conn->c_npaths = min_t(int, RDS_MPATH_WORKERS,
> -					       be16_to_cpu(buffer.rds_npaths));
> +					      (__force __u16)buffer.rds_npaths);

No.  It will break on little-endian.  That's over-the-wire data you are
dealing with; it's *NOT* going to be host-endian.  Fix the buggered
annotations instead.

>  			break;
>  		case RDS_EXTHDR_GEN_NUM:
> -			new_peer_gen_num = be32_to_cpu(buffer.rds_gen_num);
> +			new_peer_gen_num = (__force __u32)buffer.rds_gen_num;
>  			break;

Ditto.

> diff --git a/net/rds/send.c b/net/rds/send.c
> index 42d991bc8543..0d79455c9721 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -1454,8 +1454,8 @@ rds_send_probe(struct rds_conn_path *cp, __be16 sport,
>  
>  	if (RDS_HS_PROBE(be16_to_cpu(sport), be16_to_cpu(dport)) &&
>  	    cp->cp_conn->c_trans->t_mp_capable) {
> -		u16 npaths = cpu_to_be16(RDS_MPATH_WORKERS);
> -		u32 my_gen_num = cpu_to_be32(cp->cp_conn->c_my_gen_num);
> +		u16 npaths = (__force __u16)RDS_MPATH_WORKERS;
> +		u32 my_gen_num = (__force __u32)cp->cp_conn->c_my_gen_num;

Again, over-the-wire data; you are breaking it on l-e.

>  		rds_message_add_extension(&rm->m_inc.i_hdr,
>  					  RDS_EXTHDR_NPATHS, &npaths,

  parent reply	other threads:[~2025-08-10 17:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-10 17:11 [PATCH net] rds: Fix endian annotations across various assignments Ujwal Kundur
2025-08-10 17:32 ` Andrew Lunn
2025-08-10 17:47 ` Al Viro [this message]
2025-08-10 18:25   ` Al Viro
2025-08-10 18:27     ` Al Viro
2025-08-10 18:41     ` Andrew Lunn
2025-08-10 19:26       ` Al Viro
2025-08-10 19:31       ` Ujwal Kundur
2025-08-10 20:19         ` Andrew Lunn
2025-08-10 21:00         ` Al Viro
2025-08-10 21:13           ` Al Viro
2025-08-10 18:38   ` Al Viro

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=20250810174705.GK222315@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=allison.henderson@oracle.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rds-devel@oss.oracle.com \
    --cc=ujwal.kundur@gmail.com \
    /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.