Skip to contentSkip to conversation

l2tp: remove unused session fields

PATCH In review Guillaume Nault 2 participants 6 messages Jul 27, 2018

Subsystems (3)
Networking Network drivers PPP over L2TP
lore
Share & tools
lore

Guillaume Nault

author
#0 lore raw

cover l2tp: remove unused session fields

Several fields of the session structures can be set, but remain unused otherwise.
This series removes these fields and explicitely ignores the associated ioctls and netlink attributes.

Guillaume Nault (4):
l2tp: ignore L2TP_ATTR_DATA_SEQ netlink attribute
l2tp: ignore L2TP_ATTR_VLAN_ID netlink attribute
l2tp: drop ->flags from struct pppol2tp_session
l2tp: drop ->mru from struct l2tp_session

 include/uapi/linux/l2tp.h | 13 +++++-----
 net/l2tp/l2tp_core.c      |  1 -
 net/l2tp/l2tp_core.h      | 11 --------
 net/l2tp/l2tp_debugfs.c   |  6 ++---
 net/l2tp/l2tp_netlink.c   | 19 +-------------
 net/l2tp/l2tp_ppp.c       | 54 +++++----------------------------------
 6 files changed, 17 insertions(+), 87 deletions(-)
Signature · 1 line
2.18.0
author
#1 lore raw

PATCH 1/4 l2tp: ignore L2TP_ATTR_DATA_SEQ netlink attribute

The value of this attribute is never used.

Signed-off-by Guillaume Nault <[email protected]>
---
 include/uapi/linux/l2tp.h | 7 ++++---
 net/l2tp/l2tp_core.h      | 8 --------
 net/l2tp/l2tp_debugfs.c   | 4 +---
 net/l2tp/l2tp_netlink.c   | 6 ------
 4 files changed, 5 insertions(+), 20 deletions(-)
include/uapi/linux/l2tp.h +4 -3
index 7d570c7bd117..ae888606b3ec 100644
@@ -65,9 +65,9 @@ struct sockaddr_l2tpip6 {
  * TUNNEL_MODIFY	- CONN_ID, udpcsum
  * TUNNEL_GETSTATS	- CONN_ID, (stats)
  * TUNNEL_GET		- CONN_ID, (...)
- * SESSION_CREATE	- SESSION_ID, PW_TYPE, data_seq, cookie, peer_cookie, l2spec
+ * SESSION_CREATE	- SESSION_ID, PW_TYPE, cookie, peer_cookie, l2spec
  * SESSION_DELETE	- SESSION_ID
- * SESSION_MODIFY	- SESSION_ID, data_seq
+ * SESSION_MODIFY	- SESSION_ID
  * SESSION_GET		- SESSION_ID, (...)
  * SESSION_GETSTATS	- SESSION_ID, (stats)
  *

@@ -95,7 +95,7 @@ enum {
 	L2TP_ATTR_PW_TYPE,		/* u16, enum l2tp_pwtype */
 	L2TP_ATTR_ENCAP_TYPE,		/* u16, enum l2tp_encap_type */
 	L2TP_ATTR_OFFSET,		/* u16 (not used) */
-	L2TP_ATTR_DATA_SEQ,		/* u16 */
+	L2TP_ATTR_DATA_SEQ,		/* u16 (not used) */
 	L2TP_ATTR_L2SPEC_TYPE,		/* u8, enum l2tp_l2spec_type */
 	L2TP_ATTR_L2SPEC_LEN,		/* u8 (not used) */
 	L2TP_ATTR_PROTO_VERSION,	/* u8 */

@@ -169,6 +169,7 @@ enum l2tp_encap_type {
 	L2TP_ENCAPTYPE_IP,
 };
 
+/* For L2TP_ATTR_DATA_SEQ. Unused. */
 enum l2tp_seqmode {
 	L2TP_SEQ_NONE = 0,
 	L2TP_SEQ_IP = 1,
net/l2tp/l2tp_core.h +0 -8
index d85fde793a8c..7dbfb55ab3b5 100644
@@ -45,10 +45,6 @@ struct l2tp_tunnel;
  */
 struct l2tp_session_cfg {
 	enum l2tp_pwtype	pw_type;
-	unsigned int		data_seq:2;	/* data sequencing level
-						 * 0 => none, 1 => IP only,
-						 * 2 => all
-						 */
 	unsigned int		recv_seq:1;	/* expect receive packets with
 						 * sequence numbers? */
 	unsigned int		send_seq:1;	/* send packets with sequence

@@ -99,10 +95,6 @@ struct l2tp_session {
 
 	char			name[32];	/* for logging */
 	char			ifname[IFNAMSIZ];
-	unsigned int		data_seq:2;	/* data sequencing level
-						 * 0 => none, 1 => IP only,
-						 * 2 => all
-						 */
 	unsigned int		recv_seq:1;	/* expect receive packets with
 						 * sequence numbers? */
 	unsigned int		send_seq:1;	/* send packets with sequence
net/l2tp/l2tp_debugfs.c +1 -3
index b5d7dde003ef..91b9248610f0 100644
@@ -191,12 +191,10 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
 	if (session->send_seq || session->recv_seq)
 		seq_printf(m, "   nr %hu, ns %hu\n", session->nr, session->ns);
 	seq_printf(m, "   refcnt %d\n", refcount_read(&session->ref_count));
-	seq_printf(m, "   config %d/%d/%c/%c/%s/%s %08x %u\n",
+	seq_printf(m, "   config %d/%d/%c/%c/-/%s %08x %u\n",
 		   session->mtu, session->mru,
 		   session->recv_seq ? 'R' : '-',
 		   session->send_seq ? 'S' : '-',
-		   session->data_seq == 1 ? "IPSEQ" :
-		   session->data_seq == 2 ? "DATASEQ" : "-",
 		   session->lns_mode ? "LNS" : "LAC",
 		   session->debug,
 		   jiffies_to_msecs(session->reorder_timeout));
net/l2tp/l2tp_netlink.c +0 -6
index 5b9900889e31..e4785f6966f6 100644
@@ -560,9 +560,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 	}
 
 	if (tunnel->version > 2) {
-		if (info->attrs[L2TP_ATTR_DATA_SEQ])
-			cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]);
-
 		if (info->attrs[L2TP_ATTR_L2SPEC_TYPE]) {
 			cfg.l2specific_type = nla_get_u8(info->attrs[L2TP_ATTR_L2SPEC_TYPE]);
 			if (cfg.l2specific_type != L2TP_L2SPECTYPE_DEFAULT &&

@@ -693,9 +690,6 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
 	if (info->attrs[L2TP_ATTR_DEBUG])
 		session->debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]);
 
-	if (info->attrs[L2TP_ATTR_DATA_SEQ])
-		session->data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]);
-
 	if (info->attrs[L2TP_ATTR_RECV_SEQ])
 		session->recv_seq = nla_get_u8(info->attrs[L2TP_ATTR_RECV_SEQ]);
Signature · 1 line
2.18.0
author
#2 lore raw

PATCH 2/4 l2tp: ignore L2TP_ATTR_VLAN_ID netlink attribute

In reply to Guillaume Nault

The value of this attribute is never used.

Signed-off-by Guillaume Nault <[email protected]>
---
 include/uapi/linux/l2tp.h | 4 ++--
 net/l2tp/l2tp_core.h      | 1 -
 net/l2tp/l2tp_netlink.c   | 3 ---
 3 files changed, 2 insertions(+), 6 deletions(-)
include/uapi/linux/l2tp.h +2 -2
index ae888606b3ec..41bf79a4b165 100644
@@ -60,7 +60,7 @@ struct sockaddr_l2tpip6 {
 /*
  * Commands.
  * Valid TLVs of each command are:-
- * TUNNEL_CREATE	- CONN_ID, pw_type, netns, ifname, ipinfo, udpinfo, udpcsum, vlanid
+ * TUNNEL_CREATE	- CONN_ID, pw_type, netns, ifname, ipinfo, udpinfo, udpcsum
  * TUNNEL_DELETE	- CONN_ID
  * TUNNEL_MODIFY	- CONN_ID, udpcsum
  * TUNNEL_GETSTATS	- CONN_ID, (stats)

@@ -105,7 +105,7 @@ enum {
 	L2TP_ATTR_SESSION_ID,		/* u32 */
 	L2TP_ATTR_PEER_SESSION_ID,	/* u32 */
 	L2TP_ATTR_UDP_CSUM,		/* u8 */
-	L2TP_ATTR_VLAN_ID,		/* u16 */
+	L2TP_ATTR_VLAN_ID,		/* u16 (not used) */
 	L2TP_ATTR_COOKIE,		/* 0, 4 or 8 bytes */
 	L2TP_ATTR_PEER_COOKIE,		/* 0, 4 or 8 bytes */
 	L2TP_ATTR_DEBUG,		/* u32, enum l2tp_debug_flags */
net/l2tp/l2tp_core.h +0 -1
index 7dbfb55ab3b5..49fd5e05538c 100644
@@ -54,7 +54,6 @@ struct l2tp_session_cfg {
 						 * control of LNS. */
 	int			debug;		/* bitmask of debug message
 						 * categories */
-	u16			vlan_id;	/* VLAN pseudowire only */
 	u16			l2specific_type; /* Layer 2 specific type */
 	u8			cookie[8];	/* optional cookie */
 	int			cookie_len;	/* 0, 4 or 8 bytes */
net/l2tp/l2tp_netlink.c +0 -3
index e4785f6966f6..8ea1deefbc37 100644
@@ -591,9 +591,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 		}
 		if (info->attrs[L2TP_ATTR_IFNAME])
 			cfg.ifname = nla_data(info->attrs[L2TP_ATTR_IFNAME]);
-
-		if (info->attrs[L2TP_ATTR_VLAN_ID])
-			cfg.vlan_id = nla_get_u16(info->attrs[L2TP_ATTR_VLAN_ID]);
 	}
 
 	if (info->attrs[L2TP_ATTR_DEBUG])
Signature · 1 line
2.18.0
author
#3 lore raw

PATCH 3/4 l2tp: drop ->flags from struct pppol2tp_session

In reply to Guillaume Nault

This field is not used.

Keep validating user input in PPPIOCSFLAGS. Even though we discard the value, it would look wrong to succeed if an invalid address was passed from userspace.

Signed-off-by Guillaume Nault <[email protected]>
---
 net/l2tp/l2tp_ppp.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)
net/l2tp/l2tp_ppp.c +2 -11
index 000c9829304c..759ce8421269 100644
@@ -127,8 +127,6 @@ struct pppol2tp_session {
 						 * PPPoX socket */
 	struct sock		*__sk;		/* Copy of .sk, for cleanup */
 	struct rcu_head		rcu;		/* For asynchronous release */
-	int			flags;		/* accessed by PPPIOCGFLAGS.
-						 * Unused. */
 };
 
 static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb);

@@ -1057,7 +1055,6 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 	int err = 0;
 	struct sock *sk;
 	int val = (int) arg;
-	struct pppol2tp_session *ps = l2tp_session_priv(session);
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	struct pppol2tp_ioc_stats stats;
 

@@ -1134,21 +1131,15 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 
 	case PPPIOCGFLAGS:
 		err = -EFAULT;
-		if (put_user(ps->flags, (int __user *) arg))
+		if (put_user(0, (int __user *)arg))
 			break;
-
-		l2tp_info(session, L2TP_MSG_CONTROL, "%s: get flags=%d\n",
-			  session->name, ps->flags);
 		err = 0;
 		break;
 
 	case PPPIOCSFLAGS:
 		err = -EFAULT;
-		if (get_user(val, (int __user *) arg))
+		if (get_user(val, (int __user *)arg))
 			break;
-		ps->flags = val;
-		l2tp_info(session, L2TP_MSG_CONTROL, "%s: set flags=%d\n",
-			  session->name, ps->flags);
 		err = 0;
 		break;
Signature · 1 line
2.18.0
author
#4 lore raw

PATCH 4/4 l2tp: drop ->mru from struct l2tp_session

In reply to Guillaume Nault

This field is not used.

Treat PPPIOC*MRU the same way as PPPIOC*FLAGS: "get" requests return 0, while "set" requests vadidate the user supplied pointer but discard its value.

Signed-off-by Guillaume Nault <[email protected]>
---
 include/uapi/linux/l2tp.h |  2 +-
 net/l2tp/l2tp_core.c      |  1 -
 net/l2tp/l2tp_core.h      |  2 --
 net/l2tp/l2tp_debugfs.c   |  4 ++--
 net/l2tp/l2tp_netlink.c   | 10 +---------
 net/l2tp/l2tp_ppp.c       | 41 +++++----------------------------------
 6 files changed, 9 insertions(+), 51 deletions(-)
include/uapi/linux/l2tp.h +1 -1
index 41bf79a4b165..8bb8c7cfabe5 100644
@@ -120,7 +120,7 @@ enum {
 	L2TP_ATTR_UDP_SPORT,		/* u16 */
 	L2TP_ATTR_UDP_DPORT,		/* u16 */
 	L2TP_ATTR_MTU,			/* u16 */
-	L2TP_ATTR_MRU,			/* u16 */
+	L2TP_ATTR_MRU,			/* u16 (not used) */
 	L2TP_ATTR_STATS,		/* nested */
 	L2TP_ATTR_IP6_SADDR,		/* struct in6_addr */
 	L2TP_ATTR_IP6_DADDR,		/* struct in6_addr */
net/l2tp/l2tp_core.c +0 -1
index d10f4ed52d92..c61a467fd9b8 100644
@@ -1675,7 +1675,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
 			session->pwtype = cfg->pw_type;
 			session->debug = cfg->debug;
 			session->mtu = cfg->mtu;
-			session->mru = cfg->mru;
 			session->send_seq = cfg->send_seq;
 			session->recv_seq = cfg->recv_seq;
 			session->lns_mode = cfg->lns_mode;
net/l2tp/l2tp_core.h +0 -2
index 49fd5e05538c..fa5ae9432d38 100644
@@ -62,7 +62,6 @@ struct l2tp_session_cfg {
 	int			reorder_timeout; /* configured reorder timeout
 						  * (in jiffies) */
 	int			mtu;
-	int			mru;
 	char			*ifname;
 };
 

@@ -107,7 +106,6 @@ struct l2tp_session {
 						  * (in jiffies) */
 	int			reorder_skip;	/* set if skip to next nr */
 	int			mtu;
-	int			mru;
 	enum l2tp_pwtype	pwtype;
 	struct l2tp_stats	stats;
 	struct hlist_node	global_hlist;	/* Global hash list node */
net/l2tp/l2tp_debugfs.c +2 -2
index 91b9248610f0..aee271741f5b 100644
@@ -191,8 +191,8 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
 	if (session->send_seq || session->recv_seq)
 		seq_printf(m, "   nr %hu, ns %hu\n", session->nr, session->ns);
 	seq_printf(m, "   refcnt %d\n", refcount_read(&session->ref_count));
-	seq_printf(m, "   config %d/%d/%c/%c/-/%s %08x %u\n",
-		   session->mtu, session->mru,
+	seq_printf(m, "   config %d/0/%c/%c/-/%s %08x %u\n",
+		   session->mtu,
 		   session->recv_seq ? 'R' : '-',
 		   session->send_seq ? 'S' : '-',
 		   session->lns_mode ? "LNS" : "LAC",
net/l2tp/l2tp_netlink.c +1 -9
index 8ea1deefbc37..a7c409215336 100644
@@ -611,9 +611,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 	if (info->attrs[L2TP_ATTR_MTU])
 		cfg.mtu = nla_get_u16(info->attrs[L2TP_ATTR_MTU]);
 
-	if (info->attrs[L2TP_ATTR_MRU])
-		cfg.mru = nla_get_u16(info->attrs[L2TP_ATTR_MRU]);
-
 #ifdef CONFIG_MODULES
 	if (l2tp_nl_cmd_ops[cfg.pw_type] == NULL) {
 		genl_unlock();

@@ -704,9 +701,6 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
 	if (info->attrs[L2TP_ATTR_MTU])
 		session->mtu = nla_get_u16(info->attrs[L2TP_ATTR_MTU]);
 
-	if (info->attrs[L2TP_ATTR_MRU])
-		session->mru = nla_get_u16(info->attrs[L2TP_ATTR_MRU]);
-
 	ret = l2tp_session_notify(&l2tp_nl_family, info,
 				  session, L2TP_CMD_SESSION_MODIFY);
 

@@ -737,9 +731,7 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
 			session->peer_session_id) ||
 	    nla_put_u32(skb, L2TP_ATTR_DEBUG, session->debug) ||
 	    nla_put_u16(skb, L2TP_ATTR_PW_TYPE, session->pwtype) ||
-	    nla_put_u16(skb, L2TP_ATTR_MTU, session->mtu) ||
-	    (session->mru &&
-	     nla_put_u16(skb, L2TP_ATTR_MRU, session->mru)))
+	    nla_put_u16(skb, L2TP_ATTR_MTU, session->mtu))
 		goto nla_put_failure;
 
 	if ((session->ifname[0] &&
net/l2tp/l2tp_ppp.c +5 -36

78 lines not on this page · raw message

Signature · 1 line
2.18.0
maintainer
#5 lore raw
In reply to Guillaume Nault

From: Guillaume Nault <[email protected]>
Date: Fri, 27 Jul 2018 10:59:55 +0200

Quoted 4 lines

Several fields of the session structures can be set, but remain unused otherwise.
This series removes these fields and explicitely ignores the associated ioctls and netlink attributes.

Series applied, thank you.