2019-02-19 21:55:25

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] phonet: fix building with clang

clang warns about overflowing the data[] member in the struct pnpipehdr:

net/phonet/pep.c:295:8: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
if (hdr->data[4] == PEP_IND_READY)
^ ~
include/net/phonet/pep.h:66:3: note: array 'data' declared here
u8 data[1];

Using a flexible array member at the end of the struct avoids the
warning, but since we cannot have a flexible array member inside
of the union, each index now has to be moved back by one, which
makes it a little uglier.

Signed-off-by: Arnd Bergmann <[email protected]>
---
include/net/phonet/pep.h | 5 +++--
net/phonet/pep.c | 32 ++++++++++++++++----------------
2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
index b669fe6dbc3b..98f31c7ea23d 100644
--- a/include/net/phonet/pep.h
+++ b/include/net/phonet/pep.h
@@ -63,10 +63,11 @@ struct pnpipehdr {
u8 state_after_reset; /* reset request */
u8 error_code; /* any response */
u8 pep_type; /* status indication */
- u8 data[1];
+ u8 data0; /* anything else */
};
+ u8 data[];
};
-#define other_pep_type data[1]
+#define other_pep_type data[0]

static inline struct pnpipehdr *pnp_hdr(struct sk_buff *skb)
{
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 9fc76b19cd3c..db3473540303 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -132,7 +132,7 @@ static int pep_indicate(struct sock *sk, u8 id, u8 code,
ph->utid = 0;
ph->message_id = id;
ph->pipe_handle = pn->pipe_handle;
- ph->data[0] = code;
+ ph->error_code = code;
return pn_skb_send(sk, skb, NULL);
}

@@ -153,7 +153,7 @@ static int pipe_handler_request(struct sock *sk, u8 id, u8 code,
ph->utid = id; /* whatever */
ph->message_id = id;
ph->pipe_handle = pn->pipe_handle;
- ph->data[0] = code;
+ ph->error_code = code;
return pn_skb_send(sk, skb, NULL);
}

@@ -208,7 +208,7 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
struct pnpipehdr *ph;
struct sockaddr_pn dst;
u8 data[4] = {
- oph->data[0], /* PEP type */
+ oph->pep_type, /* PEP type */
code, /* error code, at an unusual offset */
PAD, PAD,
};
@@ -221,7 +221,7 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
ph->utid = oph->utid;
ph->message_id = PNS_PEP_CTRL_RESP;
ph->pipe_handle = oph->pipe_handle;
- ph->data[0] = oph->data[1]; /* CTRL id */
+ ph->data0 = oph->data[0]; /* CTRL id */

pn_skb_get_src_sockaddr(oskb, &dst);
return pn_skb_send(sk, skb, &dst);
@@ -272,17 +272,17 @@ static int pipe_rcv_status(struct sock *sk, struct sk_buff *skb)
return -EINVAL;

hdr = pnp_hdr(skb);
- if (hdr->data[0] != PN_PEP_TYPE_COMMON) {
+ if (hdr->pep_type != PN_PEP_TYPE_COMMON) {
net_dbg_ratelimited("Phonet unknown PEP type: %u\n",
- (unsigned int)hdr->data[0]);
+ (unsigned int)hdr->pep_type);
return -EOPNOTSUPP;
}

- switch (hdr->data[1]) {
+ switch (hdr->data[0]) {
case PN_PEP_IND_FLOW_CONTROL:
switch (pn->tx_fc) {
case PN_LEGACY_FLOW_CONTROL:
- switch (hdr->data[4]) {
+ switch (hdr->data[3]) {
case PEP_IND_BUSY:
atomic_set(&pn->tx_credits, 0);
break;
@@ -292,7 +292,7 @@ static int pipe_rcv_status(struct sock *sk, struct sk_buff *skb)
}
break;
case PN_ONE_CREDIT_FLOW_CONTROL:
- if (hdr->data[4] == PEP_IND_READY)
+ if (hdr->data[3] == PEP_IND_READY)
atomic_set(&pn->tx_credits, wake = 1);
break;
}
@@ -301,12 +301,12 @@ static int pipe_rcv_status(struct sock *sk, struct sk_buff *skb)
case PN_PEP_IND_ID_MCFC_GRANT_CREDITS:
if (pn->tx_fc != PN_MULTI_CREDIT_FLOW_CONTROL)
break;
- atomic_add(wake = hdr->data[4], &pn->tx_credits);
+ atomic_add(wake = hdr->data[3], &pn->tx_credits);
break;

default:
net_dbg_ratelimited("Phonet unknown PEP indication: %u\n",
- (unsigned int)hdr->data[1]);
+ (unsigned int)hdr->data[0]);
return -EOPNOTSUPP;
}
if (wake)
@@ -318,7 +318,7 @@ static int pipe_rcv_created(struct sock *sk, struct sk_buff *skb)
{
struct pep_sock *pn = pep_sk(sk);
struct pnpipehdr *hdr = pnp_hdr(skb);
- u8 n_sb = hdr->data[0];
+ u8 n_sb = hdr->data0;

pn->rx_fc = pn->tx_fc = PN_LEGACY_FLOW_CONTROL;
__skb_pull(skb, sizeof(*hdr));
@@ -506,7 +506,7 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
return -ECONNREFUSED;

/* Parse sub-blocks */
- n_sb = hdr->data[4];
+ n_sb = hdr->data[3];
while (n_sb > 0) {
u8 type, buf[6], len = sizeof(buf);
const u8 *data = pep_get_sb(skb, &type, &len, buf);
@@ -739,7 +739,7 @@ static int pipe_do_remove(struct sock *sk)
ph->utid = 0;
ph->message_id = PNS_PIPE_REMOVE_REQ;
ph->pipe_handle = pn->pipe_handle;
- ph->data[0] = PAD;
+ ph->data0 = PAD;
return pn_skb_send(sk, skb, NULL);
}

@@ -817,7 +817,7 @@ static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp,
peer_type = hdr->other_pep_type << 8;

/* Parse sub-blocks (options) */
- n_sb = hdr->data[4];
+ n_sb = hdr->data[3];
while (n_sb > 0) {
u8 type, buf[1], len = sizeof(buf);
const u8 *data = pep_get_sb(skb, &type, &len, buf);
@@ -1109,7 +1109,7 @@ static int pipe_skb_send(struct sock *sk, struct sk_buff *skb)
ph->utid = 0;
if (pn->aligned) {
ph->message_id = PNS_PIPE_ALIGNED_DATA;
- ph->data[0] = 0; /* padding */
+ ph->data0 = 0; /* padding */
} else
ph->message_id = PNS_PIPE_DATA;
ph->pipe_handle = pn->pipe_handle;
--
2.20.0



2019-02-20 15:19:28

by Rémi Denis-Courmont

[permalink] [raw]
Subject: Re: [PATCH] phonet: fix building with clang

Le tiistaina 19. helmikuuta 2019, 23.53.50 EET Arnd Bergmann a écrit :
> clang warns about overflowing the data[] member in the struct pnpipehdr:
>
> net/phonet/pep.c:295:8: warning: array index 4 is past the end of the array
> (which contains 1 element) [-Warray-bounds] if (hdr->data[4] ==
> PEP_IND_READY)
> ^ ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
> u8 data[1];
>
> Using a flexible array member at the end of the struct avoids the
> warning, but since we cannot have a flexible array member inside
> of the union, each index now has to be moved back by one, which
> makes it a little uglier.
>
> Signed-off-by: Arnd Bergmann <[email protected]>

Acked-by: Rémi Denis-Courmont <[email protected]>

> ---
> include/net/phonet/pep.h | 5 +++--
> net/phonet/pep.c | 32 ++++++++++++++++----------------
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
> index b669fe6dbc3b..98f31c7ea23d 100644
> --- a/include/net/phonet/pep.h
> +++ b/include/net/phonet/pep.h
> @@ -63,10 +63,11 @@ struct pnpipehdr {
> u8 state_after_reset; /* reset request */
> u8 error_code; /* any response */
> u8 pep_type; /* status indication */
> - u8 data[1];
> + u8 data0; /* anything else */
> };
> + u8 data[];
> };
> -#define other_pep_type data[1]
> +#define other_pep_type data[0]
>
> static inline struct pnpipehdr *pnp_hdr(struct sk_buff *skb)
> {
> diff --git a/net/phonet/pep.c b/net/phonet/pep.c
> index 9fc76b19cd3c..db3473540303 100644
> --- a/net/phonet/pep.c
> +++ b/net/phonet/pep.c
> @@ -132,7 +132,7 @@ static int pep_indicate(struct sock *sk, u8 id, u8 code,
> ph->utid = 0;
> ph->message_id = id;
> ph->pipe_handle = pn->pipe_handle;
> - ph->data[0] = code;
> + ph->error_code = code;
> return pn_skb_send(sk, skb, NULL);
> }
>
> @@ -153,7 +153,7 @@ static int pipe_handler_request(struct sock *sk, u8 id,
> u8 code, ph->utid = id; /* whatever */
> ph->message_id = id;
> ph->pipe_handle = pn->pipe_handle;
> - ph->data[0] = code;
> + ph->error_code = code;
> return pn_skb_send(sk, skb, NULL);
> }
>
> @@ -208,7 +208,7 @@ static int pep_ctrlreq_error(struct sock *sk, struct
> sk_buff *oskb, u8 code, struct pnpipehdr *ph;
> struct sockaddr_pn dst;
> u8 data[4] = {
> - oph->data[0], /* PEP type */
> + oph->pep_type, /* PEP type */
> code, /* error code, at an unusual offset */
> PAD, PAD,
> };
> @@ -221,7 +221,7 @@ static int pep_ctrlreq_error(struct sock *sk, struct
> sk_buff *oskb, u8 code, ph->utid = oph->utid;
> ph->message_id = PNS_PEP_CTRL_RESP;
> ph->pipe_handle = oph->pipe_handle;
> - ph->data[0] = oph->data[1]; /* CTRL id */
> + ph->data0 = oph->data[0]; /* CTRL id */
>
> pn_skb_get_src_sockaddr(oskb, &dst);
> return pn_skb_send(sk, skb, &dst);
> @@ -272,17 +272,17 @@ static int pipe_rcv_status(struct sock *sk, struct
> sk_buff *skb) return -EINVAL;
>
> hdr = pnp_hdr(skb);
> - if (hdr->data[0] != PN_PEP_TYPE_COMMON) {
> + if (hdr->pep_type != PN_PEP_TYPE_COMMON) {
> net_dbg_ratelimited("Phonet unknown PEP type: %u\n",
> - (unsigned int)hdr->data[0]);
> + (unsigned int)hdr->pep_type);
> return -EOPNOTSUPP;
> }
>
> - switch (hdr->data[1]) {
> + switch (hdr->data[0]) {
> case PN_PEP_IND_FLOW_CONTROL:
> switch (pn->tx_fc) {
> case PN_LEGACY_FLOW_CONTROL:
> - switch (hdr->data[4]) {
> + switch (hdr->data[3]) {
> case PEP_IND_BUSY:
> atomic_set(&pn->tx_credits, 0);
> break;
> @@ -292,7 +292,7 @@ static int pipe_rcv_status(struct sock *sk, struct
> sk_buff *skb) }
> break;
> case PN_ONE_CREDIT_FLOW_CONTROL:
> - if (hdr->data[4] == PEP_IND_READY)
> + if (hdr->data[3] == PEP_IND_READY)
> atomic_set(&pn->tx_credits, wake = 1);
> break;
> }
> @@ -301,12 +301,12 @@ static int pipe_rcv_status(struct sock *sk, struct
> sk_buff *skb) case PN_PEP_IND_ID_MCFC_GRANT_CREDITS:
> if (pn->tx_fc != PN_MULTI_CREDIT_FLOW_CONTROL)
> break;
> - atomic_add(wake = hdr->data[4], &pn->tx_credits);
> + atomic_add(wake = hdr->data[3], &pn->tx_credits);
> break;
>
> default:
> net_dbg_ratelimited("Phonet unknown PEP indication: %u\n",
> - (unsigned int)hdr->data[1]);
> + (unsigned int)hdr->data[0]);
> return -EOPNOTSUPP;
> }
> if (wake)
> @@ -318,7 +318,7 @@ static int pipe_rcv_created(struct sock *sk, struct
> sk_buff *skb) {
> struct pep_sock *pn = pep_sk(sk);
> struct pnpipehdr *hdr = pnp_hdr(skb);
> - u8 n_sb = hdr->data[0];
> + u8 n_sb = hdr->data0;
>
> pn->rx_fc = pn->tx_fc = PN_LEGACY_FLOW_CONTROL;
> __skb_pull(skb, sizeof(*hdr));
> @@ -506,7 +506,7 @@ static int pep_connresp_rcv(struct sock *sk, struct
> sk_buff *skb) return -ECONNREFUSED;
>
> /* Parse sub-blocks */
> - n_sb = hdr->data[4];
> + n_sb = hdr->data[3];
> while (n_sb > 0) {
> u8 type, buf[6], len = sizeof(buf);
> const u8 *data = pep_get_sb(skb, &type, &len, buf);
> @@ -739,7 +739,7 @@ static int pipe_do_remove(struct sock *sk)
> ph->utid = 0;
> ph->message_id = PNS_PIPE_REMOVE_REQ;
> ph->pipe_handle = pn->pipe_handle;
> - ph->data[0] = PAD;
> + ph->data0 = PAD;
> return pn_skb_send(sk, skb, NULL);
> }
>
> @@ -817,7 +817,7 @@ static struct sock *pep_sock_accept(struct sock *sk, int
> flags, int *errp, peer_type = hdr->other_pep_type << 8;
>
> /* Parse sub-blocks (options) */
> - n_sb = hdr->data[4];
> + n_sb = hdr->data[3];
> while (n_sb > 0) {
> u8 type, buf[1], len = sizeof(buf);
> const u8 *data = pep_get_sb(skb, &type, &len, buf);
> @@ -1109,7 +1109,7 @@ static int pipe_skb_send(struct sock *sk, struct
> sk_buff *skb) ph->utid = 0;
> if (pn->aligned) {
> ph->message_id = PNS_PIPE_ALIGNED_DATA;
> - ph->data[0] = 0; /* padding */
> + ph->data0 = 0; /* padding */
> } else
> ph->message_id = PNS_PIPE_DATA;
> ph->pipe_handle = pn->pipe_handle;


--
レミ・デニ-クールモン
http://www.remlab.net/




2019-02-22 00:25:15

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] phonet: fix building with clang

From: Arnd Bergmann <[email protected]>
Date: Tue, 19 Feb 2019 22:53:50 +0100

> clang warns about overflowing the data[] member in the struct pnpipehdr:
>
> net/phonet/pep.c:295:8: warning: array index 4 is past the end of the array (which contains 1 element) [-Warray-bounds]
> if (hdr->data[4] == PEP_IND_READY)
> ^ ~
> include/net/phonet/pep.h:66:3: note: array 'data' declared here
> u8 data[1];
>
> Using a flexible array member at the end of the struct avoids the
> warning, but since we cannot have a flexible array member inside
> of the union, each index now has to be moved back by one, which
> makes it a little uglier.
>
> Signed-off-by: Arnd Bergmann <[email protected]>

Applied, thank you.