* [PATCH net] sctp: Fix a big endian bug in sctp_for_each_transport()
@ 2017-09-23 10:25 Dan Carpenter
2017-09-23 11:37 ` Xin Long
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2017-09-23 10:25 UTC (permalink / raw)
To: Vlad Yasevich, Xin Long
Cc: Neil Horman, David S. Miller, linux-sctp, netdev, kernel-janitors
Fundamentally, the "pos" pointer points to "cb->args[2]" which is a long.
In the current code, we only use the high 32 bits and cast it as an
int. That works on little endian systems but will fail on big endian
systems.
Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index d7d8cba01469..7d87439f299a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -121,14 +121,14 @@ void sctp_transport_walk_stop(struct rhashtable_iter *iter);
struct sctp_transport *sctp_transport_get_next(struct net *net,
struct rhashtable_iter *iter);
struct sctp_transport *sctp_transport_get_idx(struct net *net,
- struct rhashtable_iter *iter, int pos);
+ struct rhashtable_iter *iter, long pos);
int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
struct net *net,
const union sctp_addr *laddr,
const union sctp_addr *paddr, void *p);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
int (*cb_done)(struct sctp_transport *, void *),
- struct net *net, int *pos, void *p);
+ struct net *net, long *pos, void *p);
int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
struct sctp_info *info);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d4730ada7f32..0222743b3aa8 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4603,7 +4603,7 @@ struct sctp_transport *sctp_transport_get_next(struct net *net,
struct sctp_transport *sctp_transport_get_idx(struct net *net,
struct rhashtable_iter *iter,
- int pos)
+ long pos)
{
void *obj = SEQ_START_TOKEN;
@@ -4659,7 +4659,7 @@ EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
int (*cb_done)(struct sctp_transport *, void *),
- struct net *net, int *pos, void *p) {
+ struct net *net, long *pos, void *p) {
struct rhashtable_iter hti;
struct sctp_transport *tsp;
int ret;
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index 22ed01a76b19..e9d5405aa6ac 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -493,7 +493,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
goto done;
sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
- net, (int *)&cb->args[2], &commp);
+ net, &cb->args[2], &commp);
done:
cb->args[1] = cb->args[4];
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] sctp: Fix a big endian bug in sctp_for_each_transport()
2017-09-23 10:25 [PATCH net] sctp: Fix a big endian bug in sctp_for_each_transport() Dan Carpenter
@ 2017-09-23 11:37 ` Xin Long
2017-09-25 10:19 ` [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump() Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2017-09-23 11:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp,
network dev, kernel-janitors, Marcelo Ricardo Leitner
On Sat, Sep 23, 2017 at 6:25 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Fundamentally, the "pos" pointer points to "cb->args[2]" which is a long.
> In the current code, we only use the high 32 bits and cast it as an
> int. That works on little endian systems but will fail on big endian
> systems.
>
> Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index d7d8cba01469..7d87439f299a 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -121,14 +121,14 @@ void sctp_transport_walk_stop(struct rhashtable_iter *iter);
> struct sctp_transport *sctp_transport_get_next(struct net *net,
> struct rhashtable_iter *iter);
> struct sctp_transport *sctp_transport_get_idx(struct net *net,
> - struct rhashtable_iter *iter, int pos);
> + struct rhashtable_iter *iter, long pos);
> int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
> struct net *net,
> const union sctp_addr *laddr,
> const union sctp_addr *paddr, void *p);
> int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
> int (*cb_done)(struct sctp_transport *, void *),
> - struct net *net, int *pos, void *p);
> + struct net *net, long *pos, void *p);
> int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
> int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
> struct sctp_info *info);
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d4730ada7f32..0222743b3aa8 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4603,7 +4603,7 @@ struct sctp_transport *sctp_transport_get_next(struct net *net,
>
> struct sctp_transport *sctp_transport_get_idx(struct net *net,
> struct rhashtable_iter *iter,
> - int pos)
> + long pos)
> {
> void *obj = SEQ_START_TOKEN;
>
> @@ -4659,7 +4659,7 @@ EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
>
> int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
> int (*cb_done)(struct sctp_transport *, void *),
> - struct net *net, int *pos, void *p) {
> + struct net *net, long *pos, void *p) {
> struct rhashtable_iter hti;
> struct sctp_transport *tsp;
> int ret;
> diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
> index 22ed01a76b19..e9d5405aa6ac 100644
> --- a/net/sctp/sctp_diag.c
> +++ b/net/sctp/sctp_diag.c
> @@ -493,7 +493,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> goto done;
>
> sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
> - net, (int *)&cb->args[2], &commp);
> + net, &cb->args[2], &commp);
>
> done:
> cb->args[1] = cb->args[4];
Better not to touch sctp_for_each_transport and sctp_transport_get_idx
which are supposed to be also used elsewhere as common apis.
Can you pls try to fix this in sctp_diag_dump(), like:
@@ -463,6 +463,7 @@ static void sctp_diag_dump(struct sk_buff *skb,
struct netlink_callback *cb,
.r = r,
.net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN),
};
+ int pos = cb->args[2];
/* eps hashtable dumps
* args:
@@ -493,7 +494,8 @@ static void sctp_diag_dump(struct sk_buff *skb,
struct netlink_callback *cb,
goto done;
sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
- net, (int *)&cb->args[2], &commp);
+ net, &pos, &commp);
+ cb->args[2] = pos;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump()
2017-09-23 11:37 ` Xin Long
@ 2017-09-25 10:19 ` Dan Carpenter
2017-09-25 11:23 ` Neil Horman
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-09-25 10:19 UTC (permalink / raw)
To: Vlad Yasevich, Xin Long
Cc: Neil Horman, David S. Miller, linux-sctp, netdev, kernel-janitors
The sctp_for_each_transport() function takes an pointer to int. The
cb->args[] array holds longs so it's only using the high 32 bits. It
works on little endian system but will break on big endian 64 bit
machines.
Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: The v1 patch changed the function to take a long pointer, but v2
just changes the caller.
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index 22ed01a76b19..a72a7d925d46 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -463,6 +463,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
.r = r,
.net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN),
};
+ int pos = cb->args[2];
/* eps hashtable dumps
* args:
@@ -493,7 +494,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
goto done;
sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
- net, (int *)&cb->args[2], &commp);
+ net, &pos, &commp);
+ cb->args[2] = pos;
done:
cb->args[1] = cb->args[4];
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump()
2017-09-25 10:19 ` [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump() Dan Carpenter
@ 2017-09-25 11:23 ` Neil Horman
2017-09-25 15:00 ` Xin Long
2017-09-27 4:17 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2017-09-25 11:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vlad Yasevich, Xin Long, David S. Miller, linux-sctp, netdev,
kernel-janitors
On Mon, Sep 25, 2017 at 01:19:26PM +0300, Dan Carpenter wrote:
> The sctp_for_each_transport() function takes an pointer to int. The
> cb->args[] array holds longs so it's only using the high 32 bits. It
> works on little endian system but will break on big endian 64 bit
> machines.
>
> Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The v1 patch changed the function to take a long pointer, but v2
> just changes the caller.
>
> diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
> index 22ed01a76b19..a72a7d925d46 100644
> --- a/net/sctp/sctp_diag.c
> +++ b/net/sctp/sctp_diag.c
> @@ -463,6 +463,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> .r = r,
> .net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN),
> };
> + int pos = cb->args[2];
>
> /* eps hashtable dumps
> * args:
> @@ -493,7 +494,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> goto done;
>
> sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
> - net, (int *)&cb->args[2], &commp);
> + net, &pos, &commp);
> + cb->args[2] = pos;
>
> done:
> cb->args[1] = cb->args[4];
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump()
2017-09-25 10:19 ` [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump() Dan Carpenter
2017-09-25 11:23 ` Neil Horman
@ 2017-09-25 15:00 ` Xin Long
2017-09-27 4:17 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2017-09-25 15:00 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vlad Yasevich, Neil Horman, David S. Miller, linux-sctp,
network dev, kernel-janitors
On Mon, Sep 25, 2017 at 6:19 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> The sctp_for_each_transport() function takes an pointer to int. The
> cb->args[] array holds longs so it's only using the high 32 bits. It
> works on little endian system but will break on big endian 64 bit
> machines.
>
> Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The v1 patch changed the function to take a long pointer, but v2
> just changes the caller.
>
> diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
> index 22ed01a76b19..a72a7d925d46 100644
> --- a/net/sctp/sctp_diag.c
> +++ b/net/sctp/sctp_diag.c
> @@ -463,6 +463,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> .r = r,
> .net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN),
> };
> + int pos = cb->args[2];
>
> /* eps hashtable dumps
> * args:
> @@ -493,7 +494,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> goto done;
>
> sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
> - net, (int *)&cb->args[2], &commp);
> + net, &pos, &commp);
> + cb->args[2] = pos;
>
> done:
> cb->args[1] = cb->args[4];
Reviewed-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump()
2017-09-25 10:19 ` [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump() Dan Carpenter
2017-09-25 11:23 ` Neil Horman
2017-09-25 15:00 ` Xin Long
@ 2017-09-27 4:17 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-09-27 4:17 UTC (permalink / raw)
To: dan.carpenter
Cc: vyasevich, lucien.xin, nhorman, linux-sctp, netdev,
kernel-janitors
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 25 Sep 2017 13:19:26 +0300
> The sctp_for_each_transport() function takes an pointer to int. The
> cb->args[] array holds longs so it's only using the high 32 bits. It
> works on little endian system but will break on big endian 64 bit
> machines.
>
> Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: The v1 patch changed the function to take a long pointer, but v2
> just changes the caller.
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-27 4:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-23 10:25 [PATCH net] sctp: Fix a big endian bug in sctp_for_each_transport() Dan Carpenter
2017-09-23 11:37 ` Xin Long
2017-09-25 10:19 ` [PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump() Dan Carpenter
2017-09-25 11:23 ` Neil Horman
2017-09-25 15:00 ` Xin Long
2017-09-27 4:17 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).