[PATCH 1/5] net: ipv4: using strlcpy instead of strncpy

[PATCH 1/5] net: ipv4: using strlcpy instead of strncpy

[Zhao Hongjiang]

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/ipv4/tcp_cong.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 019c238..3280234 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -170,7 +170,7 @@ void tcp_get_default_congestion_control(char *name)
 
 	rcu_read_lock();
 	ca = list_entry(tcp_cong_list.next, struct tcp_congestion_ops, list);
-	strncpy(name, ca->name, TCP_CA_NAME_MAX);
+	strlcpy(name, ca->name, sizeof(name));
 	rcu_read_unlock();
 }
 
-- 
1.7.1

[PATCH 2/5] net: netfilter: using strlcpy instead of strncpy

[Zhao Hongjiang]

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/netfilter/ipset/ip_set_core.c |   12 ++++++------
 net/netfilter/nfnetlink_acct.c    |    2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index f771390..d70895b 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1010,7 +1010,7 @@ ip_set_rename(struct sock *ctnl, struct sk_buff *skb,
 			goto out;
 		}
 	}
-	strncpy(set->name, name2, IPSET_MAXNAMELEN);
+	strlcpy(set->name, name2, sizeof(set->name));
 
 out:
 	read_unlock_bh(&ip_set_ref_lock);
@@ -1055,9 +1055,9 @@ ip_set_swap(struct sock *ctnl, struct sk_buff *skb,
 	      from->type->family == to->type->family))
 		return -IPSET_ERR_TYPE_MISMATCH;
 
-	strncpy(from_name, from->name, IPSET_MAXNAMELEN);
-	strncpy(from->name, to->name, IPSET_MAXNAMELEN);
-	strncpy(to->name, from_name, IPSET_MAXNAMELEN);
+	strlcpy(from_name, from->name, sizeof(from_name));
+	strlcpy(from->name, to->name, sizeof(from->name));
+	strlcpy(to->name, from_name, sizeof(to->name));
 
 	write_lock_bh(&ip_set_ref_lock);
 	swap(from->ref, to->ref);
@@ -1800,8 +1800,8 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 		}
 		nfnl_lock(NFNL_SUBSYS_IPSET);
 		set = nfnl_set(req_get->set.index);
-		strncpy(req_get->set.name, set ? set->name : "",
-			IPSET_MAXNAMELEN);
+		strlcpy(req_get->set.name, set ? set->name : "",
+			sizeof(req_get->set.name));
 		nfnl_unlock(NFNL_SUBSYS_IPSET);
 		goto copy;
 	}
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index c7b6d46..520247b 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -77,7 +77,7 @@ nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 	if (nfacct == NULL)
 		return -ENOMEM;
 
-	strncpy(nfacct->name, nla_data(tb[NFACCT_NAME]), NFACCT_NAME_MAX);
+	strlcpy(nfacct->name, nla_data(tb[NFACCT_NAME]), sizeof(nfacct->name));
 
 	if (tb[NFACCT_BYTES]) {
 		atomic64_set(&nfacct->bytes,
-- 
1.7.1

[PATCH 3/5] net: 8021q: using strlcpy instead of strncpy

[Zhao Hongjiang]

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/8021q/vlan_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 3a8c8fd..b2ef0bf 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -390,7 +390,7 @@ static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	struct ifreq ifrr;
 	int err = -EOPNOTSUPP;
 
-	strncpy(ifrr.ifr_name, real_dev->name, IFNAMSIZ);
+	strlcpy(ifrr.ifr_name, real_dev->name, sizeof(ifrr.ifr_name));
 	ifrr.ifr_ifru = ifr->ifr_ifru;
 
 	switch (cmd) {
-- 
1.7.1

[PATCH 4/5] net: irda: using strlcpy instead of strncpy

[Zhao Hongjiang]

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/irda/af_irda.c             |    2 +-
 net/irda/ircomm/ircomm_param.c |    3 ++-
 net/irda/irttp.c               |    2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 0578d4f..f605d23 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -480,7 +480,7 @@ static int irda_open_tsap(struct irda_sock *self, __u8 tsap_sel, char *name)
 	notify.udata_indication	     = irda_data_indication;
 	notify.flow_indication       = irda_flow_indication;
 	notify.instance = self;
-	strncpy(notify.name, name, NOTIFY_MAX_NAME);
+	strlcpy(notify.name, name, sizeof(notify.name));
 
 	self->tsap = irttp_open_tsap(tsap_sel, DEFAULT_INITIAL_CREDIT,
 				     &notify);
diff --git a/net/irda/ircomm/ircomm_param.c b/net/irda/ircomm/ircomm_param.c
index 3089391..5b80be8 100644
--- a/net/irda/ircomm/ircomm_param.c
+++ b/net/irda/ircomm/ircomm_param.c
@@ -259,7 +259,8 @@ static int ircomm_param_port_name(void *instance, irda_param_t *param, int get)
 		IRDA_DEBUG(0, "%s(), not imp!\n", __func__ );
 	} else {
 		IRDA_DEBUG(0, "%s(), port-name=%s\n", __func__ , param->pv.c);
-		strncpy(self->settings.port_name, param->pv.c, 32);
+		strlcpy(self->settings.port_name, param->pv.c,
+			sizeof(self->settings.port_name));
 	}
 
 	return 0;
diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index ae43c62..b6e1c9f 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -430,7 +430,7 @@ struct tsap_cb *irttp_open_tsap(__u8 stsap_sel, int credit, notify_t *notify)
 	if(notify->status_indication != NULL)
 		ttp_notify.status_indication = irttp_status_indication;
 	ttp_notify.instance = self;
-	strncpy(ttp_notify.name, notify->name, NOTIFY_MAX_NAME);
+	strlcpy(ttp_notify.name, notify->name, sizeof(ttp_notify.name));
 
 	self->magic = TTP_TSAP_MAGIC;
 	self->connected = FALSE;
-- 
1.7.1

[PATCH 5/5] net: bluetooth: using strlcpy instead of strncpy

[Zhao Hongjiang]

for NUL terminated string, need alway set '\0' in the end.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/bluetooth/hidp/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..828be45 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -85,7 +85,7 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
 		ci->product = session->input->id.product;
 		ci->version = session->input->id.version;
 		if (session->input->name)
-			strncpy(ci->name, session->input->name, 128);
+			strlcpy(ci->name, session->input->name, sizeof(ci->name));
 		else
 			strncpy(ci->name, "HID Boot Device", 128);
 	}
@@ -94,7 +94,7 @@ static void hidp_copy_session(struct hidp_session *session, struct hidp_conninfo
 		ci->vendor  = session->hid->vendor;
 		ci->product = session->hid->product;
 		ci->version = session->hid->version;
-		strncpy(ci->name, session->hid->name, 128);
+		strlcpy(ci->name, session->hid->name, sizeof(ci->name));
 	}
 }
 
-- 
1.7.1

RE: [PATCH 1/5] net: ipv4: using strlcpy instead of strncpy

[David Laight]

> for NUL terminated string, need alway set '\0' in the end.
> 
> Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
> ---
>  net/ipv4/tcp_cong.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
> index 019c238..3280234 100644
> --- a/net/ipv4/tcp_cong.c
> +++ b/net/ipv4/tcp_cong.c
> @@ -170,7 +170,7 @@ void tcp_get_default_congestion_control(char *name)
> 
>  	rcu_read_lock();
>  	ca = list_entry(tcp_cong_list.next, struct tcp_congestion_ops, list);
> -	strncpy(name, ca->name, TCP_CA_NAME_MAX);
> +	strlcpy(name, ca->name, sizeof(name));
>  	rcu_read_unlock();
>  }

Did you actually test this? I think not!

Blindly changing strncpy() to strlcpy() isn't a good idea.
You actually need to worry about the length of both arrays
and whether the source is always 0 terminated.
(Any whether the target is required to be terminated.)
Also, if the target might get copied to user space whether
you might be leaking uninitialized kernel memory.

The one above is just plain wrong though.

	David

Re: [PATCH 1/5] net: ipv4: using strlcpy instead of strncpy

[Eric Dumazet]

On Thu, 2013-06-20 at 18:49 +0800, Zhao Hongjiang wrote:
> for NUL terminated string, need alway set '\0' in the end.
> 
> Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
> ---
>  net/ipv4/tcp_cong.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
> index 019c238..3280234 100644
> --- a/net/ipv4/tcp_cong.c
> +++ b/net/ipv4/tcp_cong.c
> @@ -170,7 +170,7 @@ void tcp_get_default_congestion_control(char *name)
>  
>  	rcu_read_lock();
>  	ca = list_entry(tcp_cong_list.next, struct tcp_congestion_ops, list);
> -	strncpy(name, ca->name, TCP_CA_NAME_MAX);
> +	strlcpy(name, ca->name, sizeof(name));
>  	rcu_read_unlock();
>  }
>  

You are quite mistaken.

How did you test this patch exactly ?