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,
next prev 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox