* [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).