[PATCH] sctp: Add process name and pid to deprecation warnings

[PATCH] sctp: Add process name and pid to deprecation warnings

[Neil Horman]

Recently I updated the sctp socket option deprecation warnings to be both a bit
more clear and ratelimited to prevent user processes from spamming the log file.
Ben Hutchings suggested that I add the process name and pid to these warnings so
that users can tell who is responsible for using the deprecated apis.  This
patch accomplishes that.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org
---
 net/sctp/socket.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index e9c5121..d32dae7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2579,8 +2579,10 @@ static int sctp_setsockopt_delayed_ack(struct sock *sk,
 			return 0;
 	} else if (optlen == sizeof(struct sctp_assoc_value)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
-				    "Use struct sctp_sack_info instead\n");
+				    "Use struct sctp_sack_info instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&params, optval, optlen))
 			return -EFAULT;
 
@@ -2996,8 +2998,10 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 
 	if (optlen == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of int in maxseg socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&val, optval, optlen))
 			return -EFAULT;
 		params.assoc_id = 0;
@@ -3255,8 +3259,10 @@ static int sctp_setsockopt_maxburst(struct sock *sk,
 
 	if (optlen == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of int in max_burst socket option deprecated.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&val, optval, optlen))
 			return -EFAULT;
 	} else if (optlen == sizeof(struct sctp_assoc_value)) {
@@ -4577,8 +4583,10 @@ static int sctp_getsockopt_delayed_ack(struct sock *sk, int len,
 			return -EFAULT;
 	} else if (len == sizeof(struct sctp_assoc_value)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
-				    "Use struct sctp_sack_info instead\n");
+				    "Use struct sctp_sack_info instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&params, optval, len))
 			return -EFAULT;
 	} else
@@ -5223,8 +5231,10 @@ static int sctp_getsockopt_maxseg(struct sock *sk, int len,
 
 	if (len == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of int in maxseg socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		params.assoc_id = 0;
 	} else if (len >= sizeof(struct sctp_assoc_value)) {
 		len = sizeof(struct sctp_assoc_value);
@@ -5316,8 +5326,10 @@ static int sctp_getsockopt_maxburst(struct sock *sk, int len,
 
 	if (len == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (pid %d) "
 				    "Use of int in max_burst socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		params.assoc_id = 0;
 	} else if (len >= sizeof(struct sctp_assoc_value)) {
 		len = sizeof(struct sctp_assoc_value);
-- 
1.8.3.1

Re: [PATCH] sctp: Add process name and pid to deprecation warnings

[Ben Hutchings]

On Thu, 2014-01-02 at 12:54 -0500, Neil Horman wrote:
> Recently I updated the sctp socket option deprecation warnings to be both a bit
> more clear and ratelimited to prevent user processes from spamming the log file.
> Ben Hutchings suggested that I add the process name and pid to these warnings so
> that users can tell who is responsible for using the deprecated apis.  This
> patch accomplishes that.
[...]

I grepped for existing deprecation warnings that log the command and/or
PID.  The warnings are not consistent, but where the include both comand
and PID it's generally in the format "%s (%d)", comm, pid.  So please
could you follow that format?

block/scsi_ioctl.c:                     printk(KERN_WARNING "program %s is using a deprecated SCSI ioctl, please convert it to SG_IO\n", current->comm);

drivers/net/wireless/hostap/hostap_ioctl.c:     printk(KERN_DEBUG "%s: process %d (%s) used deprecated iwpriv monitor "
drivers/net/wireless/hostap/hostap_ioctl.c-            "- update software to use iwconfig mode monitor\n",
drivers/net/wireless/hostap/hostap_ioctl.c-            dev->name, task_pid_nr(current), current->comm);

drivers/scsi/scsi_ioctl.c:              printk(KERN_WARNING "program %s is using a deprecated SCSI "
drivers/scsi/scsi_ioctl.c-                     "ioctl, please convert it to SG_IO\n", current->comm);

drivers/tty/mxser.c:            printk_ratelimited(KERN_WARNING "mxser: '%s' uses deprecated ioctl "
drivers/tty/mxser.c-                                    "%x (GET_MAJOR), fix your userspace\n",
drivers/tty/mxser.c-                                    current->comm, cmd);

drivers/tty/serial/serial_core.c-                               printk(KERN_NOTICE
drivers/tty/serial/serial_core.c-                                      "%s sets custom speed on %s. This "
drivers/tty/serial/serial_core.c:                                      "is deprecated.\n", current->comm,
drivers/tty/serial/serial_core.c-                                      tty_name(port->tty, buf));

fs/hugetlbfs/inode.c-                   printk_once(KERN_WARNING
fs/hugetlbfs/inode.c:                           "%s (%d): Using mlock ulimits for SHM_HUGETLB is deprecated\n",
fs/hugetlbfs/inode.c-                           current->comm, current->pid);

fs/proc/base.c: pr_warn_once("%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
fs/proc/base.c-           current->comm, task_pid_nr(current), task_pid_nr(task),
fs/proc/base.c-           task_pid_nr(task));

kernel/capability.c:            printk(KERN_INFO "warning: `%s' uses deprecated v2"
kernel/capability.c-                   " capabilities in a way that may be insecure.\n",
kernel/capability.c-                   get_task_comm(name, current));

kernel/cgroup.c:                pr_warning("cgroup: option changes via remount are deprecated (pid=%d comm=%s)\n",
kernel/cgroup.c-                           task_tgid_nr(current), current->comm);

kernel/printk/printk.c-                 pr_warn_once("%s (%d): Attempt to access syslog with "
kernel/printk/printk.c-                              "CAP_SYS_ADMIN but no CAP_SYSLOG "
kernel/printk/printk.c:                              "(deprecated).\n",
kernel/printk/printk.c-                          current->comm, task_pid_nr(current));

kernel/sysctl_binary.c-         printk(KERN_INFO
kernel/sysctl_binary.c:                 "warning: process `%s' used the deprecated sysctl "
kernel/sysctl_binary.c-                 "system call with ", current->comm);

net/ipv6/ndisc.c:               pr_warn("process `%s' is using deprecated sysctl (%s) net.ipv6.neigh.%s.%s - use net.ipv6.neigh.%s.%s_ms instead\n",
net/ipv6/ndisc.c-                       warncomm, func,
net/ipv6/ndisc.c-                       dev_name, ctl->procname,

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] sctp: Add process name and pid to deprecation warnings

[Neil Horman]

On Thu, Jan 02, 2014 at 07:52:30PM +0000, Ben Hutchings wrote:
> On Thu, 2014-01-02 at 12:54 -0500, Neil Horman wrote:
> > Recently I updated the sctp socket option deprecation warnings to be both a bit
> > more clear and ratelimited to prevent user processes from spamming the log file.
> > Ben Hutchings suggested that I add the process name and pid to these warnings so
> > that users can tell who is responsible for using the deprecated apis.  This
> > patch accomplishes that.
> [...]
> 
> I grepped for existing deprecation warnings that log the command and/or
> PID.  The warnings are not consistent, but where the include both comand
> and PID it's generally in the format "%s (%d)", comm, pid.  So please
> could you follow that format?
> 
Thats not really true, Subsystems vary widely in how they present process
name/pid warnings.  In fact, the first example you give below that uses both
process and pid, the format is <pid> (<name>)

There are alot that use <name> (<pid>), I argee, but theres quite alot of others
as well (the lockdep subssytem uses %s/%d, and several of the arm trap handlers
actually use Process <name> (pid: <pid>).  I don't have any issue with changing
it, but I'm not sure what the advantage is if theres not going to be consistent
use throughout the kernel.

Neil

Re: [PATCH] sctp: Add process name and pid to deprecation warnings

[Ben Hutchings]

On Thu, 2014-01-02 at 15:30 -0500, Neil Horman wrote:
> On Thu, Jan 02, 2014 at 07:52:30PM +0000, Ben Hutchings wrote:
> > On Thu, 2014-01-02 at 12:54 -0500, Neil Horman wrote:
> > > Recently I updated the sctp socket option deprecation warnings to be both a bit
> > > more clear and ratelimited to prevent user processes from spamming the log file.
> > > Ben Hutchings suggested that I add the process name and pid to these warnings so
> > > that users can tell who is responsible for using the deprecated apis.  This
> > > patch accomplishes that.
> > [...]
> > 
> > I grepped for existing deprecation warnings that log the command and/or
> > PID.  The warnings are not consistent, but where the include both comand
> > and PID it's generally in the format "%s (%d)", comm, pid.  So please
> > could you follow that format?
> > 
> Thats not really true, Subsystems vary widely in how they present process
> name/pid warnings.  In fact, the first example you give below that uses both
> process and pid, the format is <pid> (<name>)
> 
> There are alot that use <name> (<pid>), I argee, but theres quite alot of others
> as well (the lockdep subssytem uses %s/%d, and several of the arm trap handlers
> actually use Process <name> (pid: <pid>).

Ah, I was just looking specifically at deprecation warnings.  I can see
that broader consistency is also worthwhile.

> I don't have any issue with changing
> it, but I'm not sure what the advantage is if theres not going to be consistent
> use throughout the kernel.

Well I thought you were trying to improve that, starting with that
DEPRECATED macro.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] sctp: Add process name and pid to deprecation warnings

[Neil Horman]

On Fri, Jan 03, 2014 at 12:18:37AM +0000, Ben Hutchings wrote:
> On Thu, 2014-01-02 at 15:30 -0500, Neil Horman wrote:
> > On Thu, Jan 02, 2014 at 07:52:30PM +0000, Ben Hutchings wrote:
> > > On Thu, 2014-01-02 at 12:54 -0500, Neil Horman wrote:
> > > > Recently I updated the sctp socket option deprecation warnings to be both a bit
> > > > more clear and ratelimited to prevent user processes from spamming the log file.
> > > > Ben Hutchings suggested that I add the process name and pid to these warnings so
> > > > that users can tell who is responsible for using the deprecated apis.  This
> > > > patch accomplishes that.
> > > [...]
> > > 
> > > I grepped for existing deprecation warnings that log the command and/or
> > > PID.  The warnings are not consistent, but where the include both comand
> > > and PID it's generally in the format "%s (%d)", comm, pid.  So please
> > > could you follow that format?
> > > 
> > Thats not really true, Subsystems vary widely in how they present process
> > name/pid warnings.  In fact, the first example you give below that uses both
> > process and pid, the format is <pid> (<name>)
> > 
> > There are alot that use <name> (<pid>), I argee, but theres quite alot of others
> > as well (the lockdep subssytem uses %s/%d, and several of the arm trap handlers
> > actually use Process <name> (pid: <pid>).
> 
> Ah, I was just looking specifically at deprecation warnings.  I can see
> that broader consistency is also worthwhile.
> 
I'm not disagreeing, just pointing out that there is no real consistency in this
format.

> > I don't have any issue with changing
> > it, but I'm not sure what the advantage is if theres not going to be consistent
> > use throughout the kernel.
> 
> Well I thought you were trying to improve that, starting with that
> DEPRECATED macro.
> 
Which is why I changed it, although I don't think theres going to be any
noticible change here unless theres a larger cross-subsystem effort undertaken
here.  Maybe I'll try to do that next time I have some free time.

Regards
Neil

> Ben.
> 
> -- 
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> 

[PATCH v2] sctp: Add process name and pid to deprecation warnings

[Neil Horman]

Recently I updated the sctp socket option deprecation warnings to be both a bit
more clear and ratelimited to prevent user processes from spamming the log file.
Ben Hutchings suggested that I add the process name and pid to these warnings so
that users can tell who is responsible for using the deprecated apis.  This
patch accomplishes that.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org

---
Change Notes
v2) Convert formatting to use  %s (%d) rather than %s (pid %d) as per request
from Ben
---
 net/sctp/socket.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index e9c5121..bcc71aa 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2579,8 +2579,10 @@ static int sctp_setsockopt_delayed_ack(struct sock *sk,
 			return 0;
 	} else if (optlen == sizeof(struct sctp_assoc_value)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
-				    "Use struct sctp_sack_info instead\n");
+				    "Use struct sctp_sack_info instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&params, optval, optlen))
 			return -EFAULT;
 
@@ -2996,8 +2998,10 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 
 	if (optlen == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of int in maxseg socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&val, optval, optlen))
 			return -EFAULT;
 		params.assoc_id = 0;
@@ -3255,8 +3259,10 @@ static int sctp_setsockopt_maxburst(struct sock *sk,
 
 	if (optlen == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of int in max_burst socket option deprecated.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&val, optval, optlen))
 			return -EFAULT;
 	} else if (optlen == sizeof(struct sctp_assoc_value)) {
@@ -4577,8 +4583,10 @@ static int sctp_getsockopt_delayed_ack(struct sock *sk, int len,
 			return -EFAULT;
 	} else if (len == sizeof(struct sctp_assoc_value)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
-				    "Use struct sctp_sack_info instead\n");
+				    "Use struct sctp_sack_info instead\n",
+				    current->comm, task_pid_nr(current));
 		if (copy_from_user(&params, optval, len))
 			return -EFAULT;
 	} else
@@ -5223,8 +5231,10 @@ static int sctp_getsockopt_maxseg(struct sock *sk, int len,
 
 	if (len == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of int in maxseg socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		params.assoc_id = 0;
 	} else if (len >= sizeof(struct sctp_assoc_value)) {
 		len = sizeof(struct sctp_assoc_value);
@@ -5316,8 +5326,10 @@ static int sctp_getsockopt_maxburst(struct sock *sk, int len,
 
 	if (len == sizeof(int)) {
 		pr_warn_ratelimited(DEPRECATED
+				    "%s (%d) "
 				    "Use of int in max_burst socket option.\n"
-				    "Use struct sctp_assoc_value instead\n");
+				    "Use struct sctp_assoc_value instead\n",
+				    current->comm, task_pid_nr(current));
 		params.assoc_id = 0;
 	} else if (len >= sizeof(struct sctp_assoc_value)) {
 		len = sizeof(struct sctp_assoc_value);
-- 
1.8.3.1

Re: [PATCH v2] sctp: Add process name and pid to deprecation warnings

[Vlad Yasevich]

On 01/02/2014 03:39 PM, Neil Horman wrote:
> Recently I updated the sctp socket option deprecation warnings to be both a bit
> more clear and ratelimited to prevent user processes from spamming the log file.
> Ben Hutchings suggested that I add the process name and pid to these warnings so
> that users can tell who is responsible for using the deprecated apis.  This
> patch accomplishes that.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Vlad Yasevich <vyasevich@gmail.com>
> CC: Ben Hutchings <bhutchings@solarflare.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: netdev@vger.kernel.org

Looks good to me, thanks.

Acked-by: Vlad Yasevich <vyasevich@gmail.com>


-vlad

> 
> ---
> Change Notes
> v2) Convert formatting to use  %s (%d) rather than %s (pid %d) as per request
> from Ben
> ---
>  net/sctp/socket.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index e9c5121..bcc71aa 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2579,8 +2579,10 @@ static int sctp_setsockopt_delayed_ack(struct sock *sk,
>  			return 0;
>  	} else if (optlen == sizeof(struct sctp_assoc_value)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
> -				    "Use struct sctp_sack_info instead\n");
> +				    "Use struct sctp_sack_info instead\n",
> +				    current->comm, task_pid_nr(current));
>  		if (copy_from_user(&params, optval, optlen))
>  			return -EFAULT;
>  
> @@ -2996,8 +2998,10 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
>  
>  	if (optlen == sizeof(int)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of int in maxseg socket option.\n"
> -				    "Use struct sctp_assoc_value instead\n");
> +				    "Use struct sctp_assoc_value instead\n",
> +				    current->comm, task_pid_nr(current));
>  		if (copy_from_user(&val, optval, optlen))
>  			return -EFAULT;
>  		params.assoc_id = 0;
> @@ -3255,8 +3259,10 @@ static int sctp_setsockopt_maxburst(struct sock *sk,
>  
>  	if (optlen == sizeof(int)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of int in max_burst socket option deprecated.\n"
> -				    "Use struct sctp_assoc_value instead\n");
> +				    "Use struct sctp_assoc_value instead\n",
> +				    current->comm, task_pid_nr(current));
>  		if (copy_from_user(&val, optval, optlen))
>  			return -EFAULT;
>  	} else if (optlen == sizeof(struct sctp_assoc_value)) {
> @@ -4577,8 +4583,10 @@ static int sctp_getsockopt_delayed_ack(struct sock *sk, int len,
>  			return -EFAULT;
>  	} else if (len == sizeof(struct sctp_assoc_value)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of struct sctp_assoc_value in delayed_ack socket option.\n"
> -				    "Use struct sctp_sack_info instead\n");
> +				    "Use struct sctp_sack_info instead\n",
> +				    current->comm, task_pid_nr(current));
>  		if (copy_from_user(&params, optval, len))
>  			return -EFAULT;
>  	} else
> @@ -5223,8 +5231,10 @@ static int sctp_getsockopt_maxseg(struct sock *sk, int len,
>  
>  	if (len == sizeof(int)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of int in maxseg socket option.\n"
> -				    "Use struct sctp_assoc_value instead\n");
> +				    "Use struct sctp_assoc_value instead\n",
> +				    current->comm, task_pid_nr(current));
>  		params.assoc_id = 0;
>  	} else if (len >= sizeof(struct sctp_assoc_value)) {
>  		len = sizeof(struct sctp_assoc_value);
> @@ -5316,8 +5326,10 @@ static int sctp_getsockopt_maxburst(struct sock *sk, int len,
>  
>  	if (len == sizeof(int)) {
>  		pr_warn_ratelimited(DEPRECATED
> +				    "%s (%d) "
>  				    "Use of int in max_burst socket option.\n"
> -				    "Use struct sctp_assoc_value instead\n");
> +				    "Use struct sctp_assoc_value instead\n",
> +				    current->comm, task_pid_nr(current));
>  		params.assoc_id = 0;
>  	} else if (len >= sizeof(struct sctp_assoc_value)) {
>  		len = sizeof(struct sctp_assoc_value);
> 

Re: [PATCH] sctp: Add process name and pid to deprecation warnings

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu,  2 Jan 2014 12:54:27 -0500

> Recently I updated the sctp socket option deprecation warnings to be both a bit
> more clear and ratelimited to prevent user processes from spamming the log file.
> Ben Hutchings suggested that I add the process name and pid to these warnings so
> that users can tell who is responsible for using the deprecated apis.  This
> patch accomplishes that.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

This is a net positive improvement, so I'm applying this.

Thanks Neil.