[PATCH 1/5] char: lp: ensure that index has not exceeded LP_NO

[PATCH 1/5] char: lp: ensure that index has not exceeded LP_NO

[Shreenidhi Shedi]

Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
---
 drivers/char/lp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 0e22e3b0a04e..d474d02b6c7f 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -972,7 +972,7 @@ static void lp_attach(struct parport *port)
 			if (port_num[i] == -1)
 				break;
 
-		if (!lp_register(i, port))
+		if (i < LP_NO && !lp_register(i, port))
 			lp_count++;
 		break;
 
-- 
2.36.1

[PATCH 2/5] char: lp: remove redundant initialization of err

[Shreenidhi Shedi]

Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
---
 drivers/char/lp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index d474d02b6c7f..536ceb11176e 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -1019,7 +1019,7 @@ static struct parport_driver lp_driver = {
 
 static int __init lp_init(void)
 {
-	int i, err = 0;
+	int i, err;
 
 	if (parport_nr[0] == LP_PARPORT_OFF)
 		return 0;
-- 
2.36.1

Re: [PATCH 2/5] char: lp: remove redundant initialization of err

[Greg KH]

On Sun, May 15, 2022 at 02:44:52AM +0530, Shreenidhi Shedi wrote:
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
> ---
>  drivers/char/lp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> index d474d02b6c7f..536ceb11176e 100644
> --- a/drivers/char/lp.c
> +++ b/drivers/char/lp.c
> @@ -1019,7 +1019,7 @@ static struct parport_driver lp_driver = {
>  
>  static int __init lp_init(void)
>  {
> -	int i, err = 0;
> +	int i, err;
>  
>  	if (parport_nr[0] == LP_PARPORT_OFF)
>  		return 0;
> -- 
> 2.36.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH 3/5] char: lp: fix most of checkspec.pl warnings & errors

[Shreenidhi Shedi]

Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
---
 drivers/char/lp.c | 339 ++++++++++++++++++++++++----------------------
 1 file changed, 178 insertions(+), 161 deletions(-)

diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 536ceb11176e..a860d22df3d4 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -16,7 +16,7 @@
  * Support for parport by Philip Blundell <philb@gnu.org>
  * Parport sharing hacking by Andrea Arcangeli
  * Fixed kernel_(to/from)_user memory copy to check for errors
- * 				by Riccardo Facchetti <fizban@tin.it>
+ *				by Riccardo Facchetti <fizban@tin.it>
  * 22-JAN-1998  Added support for devfs  Richard Gooch <rgooch@atnf.csiro.au>
  * Redesigned interrupt handling for handle printers with buggy handshake
  *				by Andrea Arcangeli, 11 May 1998
@@ -144,7 +144,7 @@ static DEFINE_MUTEX(lp_mutex);
 static struct lp_struct lp_table[LP_NO];
 static int port_num[LP_NO];
 
-static unsigned int lp_count = 0;
+static unsigned int lp_count;
 static struct class *lp_class;
 
 #ifdef CONFIG_LP_CONSOLE
@@ -161,35 +161,31 @@ static struct parport *console_registered;
 
 #define r_dtr(x)	(parport_read_data(lp_table[(x)].dev->port))
 #define r_str(x)	(parport_read_status(lp_table[(x)].dev->port))
-#define w_ctr(x,y)	do { parport_write_control(lp_table[(x)].dev->port, (y)); } while (0)
-#define w_dtr(x,y)	do { parport_write_data(lp_table[(x)].dev->port, (y)); } while (0)
+#define w_ctr(x, y)	(parport_write_control(lp_table[(x)].dev->port, (y)))
+#define w_dtr(x, y)	(parport_write_data(lp_table[(x)].dev->port, (y)))
 
 /* Claim the parport or block trying unless we've already claimed it */
 static void lp_claim_parport_or_block(struct lp_struct *this_lp)
 {
-	if (!test_and_set_bit(LP_PARPORT_CLAIMED, &this_lp->bits)) {
+	if (!test_and_set_bit(LP_PARPORT_CLAIMED, &this_lp->bits))
 		parport_claim_or_block(this_lp->dev);
-	}
 }
 
 /* Claim the parport or block trying unless we've already claimed it */
 static void lp_release_parport(struct lp_struct *this_lp)
 {
-	if (test_and_clear_bit(LP_PARPORT_CLAIMED, &this_lp->bits)) {
+	if (test_and_clear_bit(LP_PARPORT_CLAIMED, &this_lp->bits))
 		parport_release(this_lp->dev);
-	}
 }
 
-
-
 static int lp_preempt(void *handle)
 {
 	struct lp_struct *this_lp = (struct lp_struct *)handle;
+
 	set_bit(LP_PREEMPT_REQUEST, &this_lp->bits);
 	return 1;
 }
 
-
 /*
  * Try to negotiate to a new mode; if unsuccessful negotiate to
  * compatibility mode.  Return the mode we ended up in.
@@ -207,12 +203,14 @@ static int lp_negotiate(struct parport *port, int mode)
 static int lp_reset(int minor)
 {
 	int retval;
+
 	lp_claim_parport_or_block(&lp_table[minor]);
 	w_ctr(minor, LP_PSELECP);
 	udelay(LP_DELAY);
 	w_ctr(minor, LP_PSELECP | LP_PINITP);
 	retval = r_str(minor);
 	lp_release_parport(&lp_table[minor]);
+
 	return retval;
 }
 
@@ -241,30 +239,34 @@ static int lp_check_status(int minor)
 	int error = 0;
 	unsigned int last = lp_table[minor].last_error;
 	unsigned char status = r_str(minor);
+
 	if ((status & LP_PERRORP) && !(LP_F(minor) & LP_CAREFUL))
 		/* No error. */
 		last = 0;
 	else if ((status & LP_POUTPA)) {
 		if (last != LP_POUTPA) {
 			last = LP_POUTPA;
-			printk(KERN_INFO "lp%d out of paper\n", minor);
+			pr_info("lp%d out of paper\n", minor);
 		}
 		error = -ENOSPC;
 	} else if (!(status & LP_PSELECD)) {
 		if (last != LP_PSELECD) {
 			last = LP_PSELECD;
-			printk(KERN_INFO "lp%d off-line\n", minor);
+			pr_info("lp%d off-line\n", minor);
 		}
 		error = -EIO;
 	} else if (!(status & LP_PERRORP)) {
 		if (last != LP_PERRORP) {
 			last = LP_PERRORP;
-			printk(KERN_INFO "lp%d on fire\n", minor);
+			pr_info("lp%d on fire\n", minor);
 		}
 		error = -EIO;
 	} else {
-		last = 0; /* Come here if LP_CAREFUL is set and no
-			     errors are reported. */
+		/*
+		 * Come here if LP_CAREFUL is set and no
+		 * errors are reported.
+		 */
+		last = 0;
 	}
 
 	lp_table[minor].last_error = last;
@@ -280,9 +282,8 @@ static int lp_wait_ready(int minor, int nonblock)
 	int error = 0;
 
 	/* If we're not in compatibility mode, we're ready now! */
-	if (lp_table[minor].current_mode != IEEE1284_MODE_COMPAT) {
+	if (lp_table[minor].current_mode != IEEE1284_MODE_COMPAT)
 		return 0;
-	}
 
 	do {
 		error = lp_check_status(minor);
@@ -338,68 +339,69 @@ static ssize_t lp_write(struct file *file, const char __user *buf,
 			    (nonblock ? PARPORT_INACTIVITY_O_NONBLOCK
 			     : lp_table[minor].timeout));
 
-	if ((retv = lp_wait_ready(minor, nonblock)) == 0)
-	do {
-		/* Write the data. */
-		written = parport_write(port, kbuf, copy_size);
-		if (written > 0) {
-			copy_size -= written;
-			count -= written;
-			buf  += written;
-			retv += written;
-		}
+	retv = lp_wait_ready(minor, nonblock);
+	if (retv == 0)
+		do {
+			/* Write the data. */
+			written = parport_write(port, kbuf, copy_size);
+			if (written > 0) {
+				copy_size -= written;
+				count -= written;
+				buf  += written;
+				retv += written;
+			}
 
-		if (signal_pending(current)) {
-			if (retv == 0)
-				retv = -EINTR;
+			if (signal_pending(current)) {
+				if (retv == 0)
+					retv = -EINTR;
 
-			break;
-		}
+				break;
+			}
 
-		if (copy_size > 0) {
-			/* incomplete write -> check error ! */
-			int error;
+			if (copy_size > 0) {
+				/* incomplete write -> check error ! */
+				int error;
 
-			parport_negotiate(lp_table[minor].dev->port,
-					  IEEE1284_MODE_COMPAT);
-			lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
+				parport_negotiate(lp_table[minor].dev->port,
+						  IEEE1284_MODE_COMPAT);
+				lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
 
-			error = lp_wait_ready(minor, nonblock);
+				error = lp_wait_ready(minor, nonblock);
 
-			if (error) {
-				if (retv == 0)
-					retv = error;
-				break;
-			} else if (nonblock) {
-				if (retv == 0)
-					retv = -EAGAIN;
-				break;
-			}
+				if (error) {
+					if (retv == 0)
+						retv = error;
+					break;
+				} else if (nonblock) {
+					if (retv == 0)
+						retv = -EAGAIN;
+					break;
+				}
 
-			parport_yield_blocking(lp_table[minor].dev);
-			lp_table[minor].current_mode
-			  = lp_negotiate(port,
-					 lp_table[minor].best_mode);
+				parport_yield_blocking(lp_table[minor].dev);
+				lp_table[minor].current_mode
+				  = lp_negotiate(port,
+						 lp_table[minor].best_mode);
 
-		} else if (need_resched())
-			schedule();
+			} else if (need_resched())
+				schedule();
 
-		if (count) {
-			copy_size = count;
-			if (copy_size > LP_BUFFER_SIZE)
-				copy_size = LP_BUFFER_SIZE;
+			if (count) {
+				copy_size = count;
+				if (copy_size > LP_BUFFER_SIZE)
+					copy_size = LP_BUFFER_SIZE;
 
-			if (copy_from_user(kbuf, buf, copy_size)) {
-				if (retv == 0)
-					retv = -EFAULT;
-				break;
+				if (copy_from_user(kbuf, buf, copy_size)) {
+					if (retv == 0)
+						retv = -EFAULT;
+					break;
+				}
 			}
-		}
-	} while (count > 0);
+		} while (count > 0);
 
 	if (test_and_clear_bit(LP_PREEMPT_REQUEST,
 			       &lp_table[minor].bits)) {
-		printk(KERN_INFO "lp%d releasing parport\n", minor);
+		pr_info("lp%d releasing parport\n", minor);
 		parport_negotiate(lp_table[minor].dev->port,
 				  IEEE1284_MODE_COMPAT);
 		lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
@@ -418,7 +420,8 @@ static ssize_t lp_read(struct file *file, char __user *buf,
 		       size_t count, loff_t *ppos)
 {
 	DEFINE_WAIT(wait);
-	unsigned int minor=iminor(file_inode(file));
+
+	unsigned int minor = iminor(file_inode(file));
 	struct parport *port = lp_table[minor].dev->port;
 	ssize_t retval = 0;
 	char *kbuf = lp_table[minor].lp_buffer;
@@ -511,28 +514,32 @@ static int lp_open(struct inode *inode, struct file *file)
 		ret = -EBUSY;
 		goto out;
 	}
-	/* If ABORTOPEN is set and the printer is offline or out of paper,
-	   we may still want to open it to perform ioctl()s.  Therefore we
-	   have commandeered O_NONBLOCK, even though it is being used in
-	   a non-standard manner.  This is strictly a Linux hack, and
-	   should most likely only ever be used by the tunelp application. */
+	/*
+	 * If ABORTOPEN is set and the printer is offline or out of paper,
+	 * we may still want to open it to perform ioctl()s. Therefore we
+	 * have commandeered O_NONBLOCK, even though it is being used in
+	 * a non-standard manner.  This is strictly a Linux hack, and
+	 * should most likely only ever be used by the tunelp application.
+	 */
 	if ((LP_F(minor) & LP_ABORTOPEN) && !(file->f_flags & O_NONBLOCK)) {
 		int status;
+
 		lp_claim_parport_or_block(&lp_table[minor]);
 		status = r_str(minor);
 		lp_release_parport(&lp_table[minor]);
+
 		if (status & LP_POUTPA) {
-			printk(KERN_INFO "lp%d out of paper\n", minor);
+			pr_info("lp%d out of paper\n", minor);
 			LP_F(minor) &= ~LP_BUSY;
 			ret = -ENOSPC;
 			goto out;
 		} else if (!(status & LP_PSELECD)) {
-			printk(KERN_INFO "lp%d off-line\n", minor);
+			pr_info("lp%d off-line\n", minor);
 			LP_F(minor) &= ~LP_BUSY;
 			ret = -EIO;
 			goto out;
 		} else if (!(status & LP_PERRORP)) {
-			printk(KERN_ERR "lp%d printer error\n", minor);
+			pr_err("lp%d printer error\n", minor);
 			LP_F(minor) &= ~LP_BUSY;
 			ret = -EIO;
 			goto out;
@@ -549,7 +556,7 @@ static int lp_open(struct inode *inode, struct file *file)
 	if ((lp_table[minor].dev->port->modes & PARPORT_MODE_ECP) &&
 	     !parport_negotiate(lp_table[minor].dev->port,
 				 IEEE1284_MODE_ECP)) {
-		printk(KERN_INFO "lp%d: ECP mode\n", minor);
+		pr_info("lp%d: ECP mode\n", minor);
 		lp_table[minor].best_mode = IEEE1284_MODE_ECP;
 	} else {
 		lp_table[minor].best_mode = IEEE1284_MODE_COMPAT;
@@ -584,81 +591,81 @@ static int lp_do_ioctl(unsigned int minor, unsigned int cmd,
 	int retval = 0;
 
 #ifdef LP_DEBUG
-	printk(KERN_DEBUG "lp%d ioctl, cmd: 0x%x, arg: 0x%lx\n", minor, cmd, arg);
+	pr_debug("lp%d ioctl, cmd: 0x%x, arg: 0x%lx\n", minor, cmd, arg);
 #endif
 	if (minor >= LP_NO)
 		return -ENODEV;
 	if ((LP_F(minor) & LP_EXIST) == 0)
 		return -ENODEV;
 	switch (cmd) {
-		case LPTIME:
-			if (arg > UINT_MAX / HZ)
-				return -EINVAL;
-			LP_TIME(minor) = arg * HZ/100;
-			break;
-		case LPCHAR:
-			LP_CHAR(minor) = arg;
-			break;
-		case LPABORT:
-			if (arg)
-				LP_F(minor) |= LP_ABORT;
-			else
-				LP_F(minor) &= ~LP_ABORT;
-			break;
-		case LPABORTOPEN:
-			if (arg)
-				LP_F(minor) |= LP_ABORTOPEN;
-			else
-				LP_F(minor) &= ~LP_ABORTOPEN;
-			break;
-		case LPCAREFUL:
-			if (arg)
-				LP_F(minor) |= LP_CAREFUL;
-			else
-				LP_F(minor) &= ~LP_CAREFUL;
-			break;
-		case LPWAIT:
-			LP_WAIT(minor) = arg;
-			break;
-		case LPSETIRQ:
+	case LPTIME:
+		if (arg > UINT_MAX / HZ)
 			return -EINVAL;
-		case LPGETIRQ:
-			if (copy_to_user(argp, &LP_IRQ(minor),
-					sizeof(int)))
-				return -EFAULT;
-			break;
-		case LPGETSTATUS:
-			if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
-				return -EINTR;
-			lp_claim_parport_or_block(&lp_table[minor]);
-			status = r_str(minor);
-			lp_release_parport(&lp_table[minor]);
-			mutex_unlock(&lp_table[minor].port_mutex);
-
-			if (copy_to_user(argp, &status, sizeof(int)))
-				return -EFAULT;
-			break;
-		case LPRESET:
-			lp_reset(minor);
-			break;
+		LP_TIME(minor) = arg * HZ/100;
+		break;
+	case LPCHAR:
+		LP_CHAR(minor) = arg;
+		break;
+	case LPABORT:
+		if (arg)
+			LP_F(minor) |= LP_ABORT;
+		else
+			LP_F(minor) &= ~LP_ABORT;
+		break;
+	case LPABORTOPEN:
+		if (arg)
+			LP_F(minor) |= LP_ABORTOPEN;
+		else
+			LP_F(minor) &= ~LP_ABORTOPEN;
+		break;
+	case LPCAREFUL:
+		if (arg)
+			LP_F(minor) |= LP_CAREFUL;
+		else
+			LP_F(minor) &= ~LP_CAREFUL;
+		break;
+	case LPWAIT:
+		LP_WAIT(minor) = arg;
+		break;
+	case LPSETIRQ:
+		return -EINVAL;
+	case LPGETIRQ:
+		if (copy_to_user(argp, &LP_IRQ(minor),
+				sizeof(int)))
+			return -EFAULT;
+		break;
+	case LPGETSTATUS:
+		if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
+			return -EINTR;
+		lp_claim_parport_or_block(&lp_table[minor]);
+		status = r_str(minor);
+		lp_release_parport(&lp_table[minor]);
+		mutex_unlock(&lp_table[minor].port_mutex);
+
+		if (copy_to_user(argp, &status, sizeof(int)))
+			return -EFAULT;
+		break;
+	case LPRESET:
+		lp_reset(minor);
+		break;
 #ifdef LP_STATS
-		case LPGETSTATS:
-			if (copy_to_user(argp, &LP_STAT(minor),
-					sizeof(struct lp_stats)))
-				return -EFAULT;
-			if (capable(CAP_SYS_ADMIN))
-				memset(&LP_STAT(minor), 0,
-						sizeof(struct lp_stats));
-			break;
+	case LPGETSTATS:
+		if (copy_to_user(argp, &LP_STAT(minor),
+				sizeof(struct lp_stats)))
+			return -EFAULT;
+		if (capable(CAP_SYS_ADMIN))
+			memset(&LP_STAT(minor), 0,
+					sizeof(struct lp_stats));
+		break;
 #endif
-		case LPGETFLAGS:
-			status = LP_F(minor);
-			if (copy_to_user(argp, &status, sizeof(int)))
-				return -EFAULT;
-			break;
+	case LPGETFLAGS:
+		status = LP_F(minor);
+		if (copy_to_user(argp, &status, sizeof(int)))
+			return -EFAULT;
+		break;
 
-		default:
-			retval = -EINVAL;
+	default:
+		retval = -EINVAL;
 	}
 	return retval;
 }
@@ -688,9 +695,9 @@ static int lp_set_timeout(unsigned int minor, s64 tv_sec, long tv_usec)
 		to_jiffies += tv_sec * (long) HZ;
 	}
 
-	if (to_jiffies <= 0) {
+	if (to_jiffies <= 0)
 		return -EINVAL;
-	}
+
 	lp_table[minor].timeout = to_jiffies;
 	return 0;
 }
@@ -802,15 +809,17 @@ static const struct file_operations lp_fops = {
 
 #define CONSOLE_LP 0
 
-/* If the printer is out of paper, we can either lose the messages or
+/*
+ * If the printer is out of paper, we can either lose the messages or
  * stall until the printer is happy again.  Define CONSOLE_LP_STRICT
- * non-zero to get the latter behaviour. */
+ * non-zero to get the latter behaviour.
+ */
 #define CONSOLE_LP_STRICT 1
 
 /* The console must be locked when we get here. */
 
 static void lp_console_write(struct console *co, const char *s,
-			     unsigned count)
+			     unsigned int count)
 {
 	struct pardevice *dev = lp_table[CONSOLE_LP].dev;
 	struct parport *port = dev->port;
@@ -829,6 +838,7 @@ static void lp_console_write(struct console *co, const char *s,
 		/* Write the data, converting LF->CRLF as we go. */
 		ssize_t canwrite = count;
 		char *lf = memchr(s, '\n', count);
+
 		if (lf)
 			canwrite = lf - s;
 
@@ -891,24 +901,28 @@ static int __init lp_setup(char *str)
 			/* disable driver on "lp=" or "lp=0" */
 			parport_nr[0] = LP_PARPORT_OFF;
 		} else {
-			printk(KERN_WARNING "warning: 'lp=0x%x' is deprecated, ignored\n", x);
+			pr_warn("warning: 'lp=0x%x' is deprecated, ignored\n", x);
 			return 0;
 		}
 	} else if (!strncmp(str, "parport", 7)) {
-		int n = simple_strtoul(str+7, NULL, 10);
+		int err;
+		unsigned long n = 0;
+
+		err = kstrtoul(str+7, 10, &n);
+		if (err)
+			return err;
+
 		if (parport_ptr < LP_NO)
 			parport_nr[parport_ptr++] = n;
 		else
-			printk(KERN_INFO "lp: too many ports, %s ignored.\n",
-			       str);
+			pr_info("lp: too many ports, %s ignored.\n", str);
 	} else if (!strcmp(str, "auto")) {
 		parport_nr[0] = LP_PARPORT_AUTO;
 	} else if (!strcmp(str, "none")) {
 		if (parport_ptr < LP_NO)
 			parport_nr[parport_ptr++] = LP_PARPORT_NONE;
 		else
-			printk(KERN_INFO "lp: too many ports, %s ignored.\n",
-			       str);
+			pr_info("lp: too many ports, %s ignored.\n", str);
 	} else if (!strcmp(str, "reset")) {
 		reset = true;
 	}
@@ -935,7 +949,7 @@ static int lp_register(int nr, struct parport *port)
 	device_create(lp_class, port->dev, MKDEV(LP_MAJOR, nr), NULL,
 		      "lp%d", nr);
 
-	printk(KERN_INFO "lp%d: using %s (%s).\n", nr, port->name,
+	pr_info("lp%d: using %s (%s).\n", nr, port->name,
 	       (port->irq == PARPORT_IRQ_NONE)?"polling":"interrupt-driven");
 
 #ifdef CONFIG_LP_CONSOLE
@@ -943,9 +957,9 @@ static int lp_register(int nr, struct parport *port)
 		if (port->modes & PARPORT_MODE_SAFEININT) {
 			register_console(&lpcons);
 			console_registered = port;
-			printk(KERN_INFO "lp%d: console ready\n", CONSOLE_LP);
+			pr_info("lp%d: console ready\n", CONSOLE_LP);
 		} else
-			printk(KERN_ERR "lp%d: cannot run console on %s\n",
+			pr_err("lp%d: cannot run console on %s\n",
 			       CONSOLE_LP, port->name);
 	}
 #endif
@@ -965,7 +979,7 @@ static void lp_attach(struct parport *port)
 		    port->probe_info[0].class != PARPORT_CLASS_PRINTER)
 			return;
 		if (lp_count == LP_NO) {
-			printk(KERN_INFO "lp: ignoring parallel port (max. %d)\n",LP_NO);
+			pr_info("lp: ignoring parallel port (max. %d)\n", LP_NO);
 			return;
 		}
 		for (i = 0; i < LP_NO; i++)
@@ -1045,7 +1059,7 @@ static int __init lp_init(void)
 	}
 
 	if (register_chrdev(LP_MAJOR, "lp", &lp_fops)) {
-		printk(KERN_ERR "lp: unable to get major %d\n", LP_MAJOR);
+		pr_err("lp: unable to get major %d\n", LP_MAJOR);
 		return -EIO;
 	}
 
@@ -1056,16 +1070,16 @@ static int __init lp_init(void)
 	}
 
 	if (parport_register_driver(&lp_driver)) {
-		printk(KERN_ERR "lp: unable to register with parport\n");
+		pr_err("lp: unable to register with parport\n");
 		err = -EIO;
 		goto out_class;
 	}
 
 	if (!lp_count) {
-		printk(KERN_INFO "lp: driver loaded but no devices found\n");
+		pr_info("lp: driver loaded but no devices found\n");
 #ifndef CONFIG_PARPORT_1284
 		if (parport_nr[0] == LP_PARPORT_AUTO)
-			printk(KERN_INFO "lp: (is IEEE 1284 support enabled?)\n");
+			pr_info("lp: (is IEEE 1284 support enabled?)\n");
 #endif
 	}
 
@@ -1086,16 +1100,19 @@ static int __init lp_init_module(void)
 			parport_nr[0] = LP_PARPORT_AUTO;
 		else {
 			int n;
+
 			for (n = 0; n < LP_NO && parport[n]; n++) {
 				if (!strncmp(parport[n], "none", 4))
 					parport_nr[n] = LP_PARPORT_NONE;
 				else {
 					char *ep;
+					/* Don't update to kstrtoul, ep needs to be updated */
 					unsigned long r = simple_strtoul(parport[n], &ep, 0);
+
 					if (ep != parport[n])
 						parport_nr[n] = r;
 					else {
-						printk(KERN_ERR "lp: bad port specifier `%s'\n", parport[n]);
+						pr_err("lp: bad port specifier `%s'\n", parport[n]);
 						return -ENODEV;
 					}
 				}
-- 
2.36.1

Re: [PATCH 3/5] char: lp: fix most of checkspec.pl warnings & errors

[Greg KH]

On Sun, May 15, 2022 at 02:44:53AM +0530, Shreenidhi Shedi wrote:
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
> ---
>  drivers/char/lp.c | 339 ++++++++++++++++++++++++----------------------
>  1 file changed, 178 insertions(+), 161 deletions(-)
> 
> diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> index 536ceb11176e..a860d22df3d4 100644
> --- a/drivers/char/lp.c
> +++ b/drivers/char/lp.c
> @@ -16,7 +16,7 @@
>   * Support for parport by Philip Blundell <philb@gnu.org>
>   * Parport sharing hacking by Andrea Arcangeli
>   * Fixed kernel_(to/from)_user memory copy to check for errors
> - * 				by Riccardo Facchetti <fizban@tin.it>
> + *				by Riccardo Facchetti <fizban@tin.it>
>   * 22-JAN-1998  Added support for devfs  Richard Gooch <rgooch@atnf.csiro.au>
>   * Redesigned interrupt handling for handle printers with buggy handshake
>   *				by Andrea Arcangeli, 11 May 1998
> @@ -144,7 +144,7 @@ static DEFINE_MUTEX(lp_mutex);
>  static struct lp_struct lp_table[LP_NO];
>  static int port_num[LP_NO];
>  
> -static unsigned int lp_count = 0;
> +static unsigned int lp_count;
>  static struct class *lp_class;
>  
>  #ifdef CONFIG_LP_CONSOLE
> @@ -161,35 +161,31 @@ static struct parport *console_registered;
>  
>  #define r_dtr(x)	(parport_read_data(lp_table[(x)].dev->port))
>  #define r_str(x)	(parport_read_status(lp_table[(x)].dev->port))
> -#define w_ctr(x,y)	do { parport_write_control(lp_table[(x)].dev->port, (y)); } while (0)
> -#define w_dtr(x,y)	do { parport_write_data(lp_table[(x)].dev->port, (y)); } while (0)
> +#define w_ctr(x, y)	(parport_write_control(lp_table[(x)].dev->port, (y)))
> +#define w_dtr(x, y)	(parport_write_data(lp_table[(x)].dev->port, (y)))
>  
>  /* Claim the parport or block trying unless we've already claimed it */
>  static void lp_claim_parport_or_block(struct lp_struct *this_lp)
>  {
> -	if (!test_and_set_bit(LP_PARPORT_CLAIMED, &this_lp->bits)) {
> +	if (!test_and_set_bit(LP_PARPORT_CLAIMED, &this_lp->bits))
>  		parport_claim_or_block(this_lp->dev);
> -	}
>  }
>  
>  /* Claim the parport or block trying unless we've already claimed it */
>  static void lp_release_parport(struct lp_struct *this_lp)
>  {
> -	if (test_and_clear_bit(LP_PARPORT_CLAIMED, &this_lp->bits)) {
> +	if (test_and_clear_bit(LP_PARPORT_CLAIMED, &this_lp->bits))
>  		parport_release(this_lp->dev);
> -	}
>  }
>  
> -
> -
>  static int lp_preempt(void *handle)
>  {
>  	struct lp_struct *this_lp = (struct lp_struct *)handle;
> +
>  	set_bit(LP_PREEMPT_REQUEST, &this_lp->bits);
>  	return 1;
>  }
>  
> -
>  /*
>   * Try to negotiate to a new mode; if unsuccessful negotiate to
>   * compatibility mode.  Return the mode we ended up in.
> @@ -207,12 +203,14 @@ static int lp_negotiate(struct parport *port, int mode)
>  static int lp_reset(int minor)
>  {
>  	int retval;
> +
>  	lp_claim_parport_or_block(&lp_table[minor]);
>  	w_ctr(minor, LP_PSELECP);
>  	udelay(LP_DELAY);
>  	w_ctr(minor, LP_PSELECP | LP_PINITP);
>  	retval = r_str(minor);
>  	lp_release_parport(&lp_table[minor]);
> +
>  	return retval;
>  }
>  
> @@ -241,30 +239,34 @@ static int lp_check_status(int minor)
>  	int error = 0;
>  	unsigned int last = lp_table[minor].last_error;
>  	unsigned char status = r_str(minor);
> +
>  	if ((status & LP_PERRORP) && !(LP_F(minor) & LP_CAREFUL))
>  		/* No error. */
>  		last = 0;
>  	else if ((status & LP_POUTPA)) {
>  		if (last != LP_POUTPA) {
>  			last = LP_POUTPA;
> -			printk(KERN_INFO "lp%d out of paper\n", minor);
> +			pr_info("lp%d out of paper\n", minor);
>  		}
>  		error = -ENOSPC;
>  	} else if (!(status & LP_PSELECD)) {
>  		if (last != LP_PSELECD) {
>  			last = LP_PSELECD;
> -			printk(KERN_INFO "lp%d off-line\n", minor);
> +			pr_info("lp%d off-line\n", minor);
>  		}
>  		error = -EIO;
>  	} else if (!(status & LP_PERRORP)) {
>  		if (last != LP_PERRORP) {
>  			last = LP_PERRORP;
> -			printk(KERN_INFO "lp%d on fire\n", minor);
> +			pr_info("lp%d on fire\n", minor);
>  		}
>  		error = -EIO;
>  	} else {
> -		last = 0; /* Come here if LP_CAREFUL is set and no
> -			     errors are reported. */
> +		/*
> +		 * Come here if LP_CAREFUL is set and no
> +		 * errors are reported.
> +		 */
> +		last = 0;
>  	}
>  
>  	lp_table[minor].last_error = last;
> @@ -280,9 +282,8 @@ static int lp_wait_ready(int minor, int nonblock)
>  	int error = 0;
>  
>  	/* If we're not in compatibility mode, we're ready now! */
> -	if (lp_table[minor].current_mode != IEEE1284_MODE_COMPAT) {
> +	if (lp_table[minor].current_mode != IEEE1284_MODE_COMPAT)
>  		return 0;
> -	}
>  
>  	do {
>  		error = lp_check_status(minor);
> @@ -338,68 +339,69 @@ static ssize_t lp_write(struct file *file, const char __user *buf,
>  			    (nonblock ? PARPORT_INACTIVITY_O_NONBLOCK
>  			     : lp_table[minor].timeout));
>  
> -	if ((retv = lp_wait_ready(minor, nonblock)) == 0)
> -	do {
> -		/* Write the data. */
> -		written = parport_write(port, kbuf, copy_size);
> -		if (written > 0) {
> -			copy_size -= written;
> -			count -= written;
> -			buf  += written;
> -			retv += written;
> -		}
> +	retv = lp_wait_ready(minor, nonblock);
> +	if (retv == 0)
> +		do {
> +			/* Write the data. */
> +			written = parport_write(port, kbuf, copy_size);
> +			if (written > 0) {
> +				copy_size -= written;
> +				count -= written;
> +				buf  += written;
> +				retv += written;
> +			}
>  
> -		if (signal_pending(current)) {
> -			if (retv == 0)
> -				retv = -EINTR;
> +			if (signal_pending(current)) {
> +				if (retv == 0)
> +					retv = -EINTR;
>  
> -			break;
> -		}
> +				break;
> +			}
>  
> -		if (copy_size > 0) {
> -			/* incomplete write -> check error ! */
> -			int error;
> +			if (copy_size > 0) {
> +				/* incomplete write -> check error ! */
> +				int error;
>  
> -			parport_negotiate(lp_table[minor].dev->port,
> -					  IEEE1284_MODE_COMPAT);
> -			lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
> +				parport_negotiate(lp_table[minor].dev->port,
> +						  IEEE1284_MODE_COMPAT);
> +				lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
>  
> -			error = lp_wait_ready(minor, nonblock);
> +				error = lp_wait_ready(minor, nonblock);
>  
> -			if (error) {
> -				if (retv == 0)
> -					retv = error;
> -				break;
> -			} else if (nonblock) {
> -				if (retv == 0)
> -					retv = -EAGAIN;
> -				break;
> -			}
> +				if (error) {
> +					if (retv == 0)
> +						retv = error;
> +					break;
> +				} else if (nonblock) {
> +					if (retv == 0)
> +						retv = -EAGAIN;
> +					break;
> +				}
>  
> -			parport_yield_blocking(lp_table[minor].dev);
> -			lp_table[minor].current_mode
> -			  = lp_negotiate(port,
> -					 lp_table[minor].best_mode);
> +				parport_yield_blocking(lp_table[minor].dev);
> +				lp_table[minor].current_mode
> +				  = lp_negotiate(port,
> +						 lp_table[minor].best_mode);
>  
> -		} else if (need_resched())
> -			schedule();
> +			} else if (need_resched())
> +				schedule();
>  
> -		if (count) {
> -			copy_size = count;
> -			if (copy_size > LP_BUFFER_SIZE)
> -				copy_size = LP_BUFFER_SIZE;
> +			if (count) {
> +				copy_size = count;
> +				if (copy_size > LP_BUFFER_SIZE)
> +					copy_size = LP_BUFFER_SIZE;
>  
> -			if (copy_from_user(kbuf, buf, copy_size)) {
> -				if (retv == 0)
> -					retv = -EFAULT;
> -				break;
> +				if (copy_from_user(kbuf, buf, copy_size)) {
> +					if (retv == 0)
> +						retv = -EFAULT;
> +					break;
> +				}
>  			}
> -		}
> -	} while (count > 0);
> +		} while (count > 0);
>  
>  	if (test_and_clear_bit(LP_PREEMPT_REQUEST,
>  			       &lp_table[minor].bits)) {
> -		printk(KERN_INFO "lp%d releasing parport\n", minor);
> +		pr_info("lp%d releasing parport\n", minor);
>  		parport_negotiate(lp_table[minor].dev->port,
>  				  IEEE1284_MODE_COMPAT);
>  		lp_table[minor].current_mode = IEEE1284_MODE_COMPAT;
> @@ -418,7 +420,8 @@ static ssize_t lp_read(struct file *file, char __user *buf,
>  		       size_t count, loff_t *ppos)
>  {
>  	DEFINE_WAIT(wait);
> -	unsigned int minor=iminor(file_inode(file));
> +
> +	unsigned int minor = iminor(file_inode(file));
>  	struct parport *port = lp_table[minor].dev->port;
>  	ssize_t retval = 0;
>  	char *kbuf = lp_table[minor].lp_buffer;
> @@ -511,28 +514,32 @@ static int lp_open(struct inode *inode, struct file *file)
>  		ret = -EBUSY;
>  		goto out;
>  	}
> -	/* If ABORTOPEN is set and the printer is offline or out of paper,
> -	   we may still want to open it to perform ioctl()s.  Therefore we
> -	   have commandeered O_NONBLOCK, even though it is being used in
> -	   a non-standard manner.  This is strictly a Linux hack, and
> -	   should most likely only ever be used by the tunelp application. */
> +	/*
> +	 * If ABORTOPEN is set and the printer is offline or out of paper,
> +	 * we may still want to open it to perform ioctl()s. Therefore we
> +	 * have commandeered O_NONBLOCK, even though it is being used in
> +	 * a non-standard manner.  This is strictly a Linux hack, and
> +	 * should most likely only ever be used by the tunelp application.
> +	 */
>  	if ((LP_F(minor) & LP_ABORTOPEN) && !(file->f_flags & O_NONBLOCK)) {
>  		int status;
> +
>  		lp_claim_parport_or_block(&lp_table[minor]);
>  		status = r_str(minor);
>  		lp_release_parport(&lp_table[minor]);
> +
>  		if (status & LP_POUTPA) {
> -			printk(KERN_INFO "lp%d out of paper\n", minor);
> +			pr_info("lp%d out of paper\n", minor);
>  			LP_F(minor) &= ~LP_BUSY;
>  			ret = -ENOSPC;
>  			goto out;
>  		} else if (!(status & LP_PSELECD)) {
> -			printk(KERN_INFO "lp%d off-line\n", minor);
> +			pr_info("lp%d off-line\n", minor);
>  			LP_F(minor) &= ~LP_BUSY;
>  			ret = -EIO;
>  			goto out;
>  		} else if (!(status & LP_PERRORP)) {
> -			printk(KERN_ERR "lp%d printer error\n", minor);
> +			pr_err("lp%d printer error\n", minor);
>  			LP_F(minor) &= ~LP_BUSY;
>  			ret = -EIO;
>  			goto out;
> @@ -549,7 +556,7 @@ static int lp_open(struct inode *inode, struct file *file)
>  	if ((lp_table[minor].dev->port->modes & PARPORT_MODE_ECP) &&
>  	     !parport_negotiate(lp_table[minor].dev->port,
>  				 IEEE1284_MODE_ECP)) {
> -		printk(KERN_INFO "lp%d: ECP mode\n", minor);
> +		pr_info("lp%d: ECP mode\n", minor);
>  		lp_table[minor].best_mode = IEEE1284_MODE_ECP;
>  	} else {
>  		lp_table[minor].best_mode = IEEE1284_MODE_COMPAT;
> @@ -584,81 +591,81 @@ static int lp_do_ioctl(unsigned int minor, unsigned int cmd,
>  	int retval = 0;
>  
>  #ifdef LP_DEBUG
> -	printk(KERN_DEBUG "lp%d ioctl, cmd: 0x%x, arg: 0x%lx\n", minor, cmd, arg);
> +	pr_debug("lp%d ioctl, cmd: 0x%x, arg: 0x%lx\n", minor, cmd, arg);
>  #endif
>  	if (minor >= LP_NO)
>  		return -ENODEV;
>  	if ((LP_F(minor) & LP_EXIST) == 0)
>  		return -ENODEV;
>  	switch (cmd) {
> -		case LPTIME:
> -			if (arg > UINT_MAX / HZ)
> -				return -EINVAL;
> -			LP_TIME(minor) = arg * HZ/100;
> -			break;
> -		case LPCHAR:
> -			LP_CHAR(minor) = arg;
> -			break;
> -		case LPABORT:
> -			if (arg)
> -				LP_F(minor) |= LP_ABORT;
> -			else
> -				LP_F(minor) &= ~LP_ABORT;
> -			break;
> -		case LPABORTOPEN:
> -			if (arg)
> -				LP_F(minor) |= LP_ABORTOPEN;
> -			else
> -				LP_F(minor) &= ~LP_ABORTOPEN;
> -			break;
> -		case LPCAREFUL:
> -			if (arg)
> -				LP_F(minor) |= LP_CAREFUL;
> -			else
> -				LP_F(minor) &= ~LP_CAREFUL;
> -			break;
> -		case LPWAIT:
> -			LP_WAIT(minor) = arg;
> -			break;
> -		case LPSETIRQ:
> +	case LPTIME:
> +		if (arg > UINT_MAX / HZ)
>  			return -EINVAL;
> -		case LPGETIRQ:
> -			if (copy_to_user(argp, &LP_IRQ(minor),
> -					sizeof(int)))
> -				return -EFAULT;
> -			break;
> -		case LPGETSTATUS:
> -			if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
> -				return -EINTR;
> -			lp_claim_parport_or_block(&lp_table[minor]);
> -			status = r_str(minor);
> -			lp_release_parport(&lp_table[minor]);
> -			mutex_unlock(&lp_table[minor].port_mutex);
> -
> -			if (copy_to_user(argp, &status, sizeof(int)))
> -				return -EFAULT;
> -			break;
> -		case LPRESET:
> -			lp_reset(minor);
> -			break;
> +		LP_TIME(minor) = arg * HZ/100;
> +		break;
> +	case LPCHAR:
> +		LP_CHAR(minor) = arg;
> +		break;
> +	case LPABORT:
> +		if (arg)
> +			LP_F(minor) |= LP_ABORT;
> +		else
> +			LP_F(minor) &= ~LP_ABORT;
> +		break;
> +	case LPABORTOPEN:
> +		if (arg)
> +			LP_F(minor) |= LP_ABORTOPEN;
> +		else
> +			LP_F(minor) &= ~LP_ABORTOPEN;
> +		break;
> +	case LPCAREFUL:
> +		if (arg)
> +			LP_F(minor) |= LP_CAREFUL;
> +		else
> +			LP_F(minor) &= ~LP_CAREFUL;
> +		break;
> +	case LPWAIT:
> +		LP_WAIT(minor) = arg;
> +		break;
> +	case LPSETIRQ:
> +		return -EINVAL;
> +	case LPGETIRQ:
> +		if (copy_to_user(argp, &LP_IRQ(minor),
> +				sizeof(int)))
> +			return -EFAULT;
> +		break;
> +	case LPGETSTATUS:
> +		if (mutex_lock_interruptible(&lp_table[minor].port_mutex))
> +			return -EINTR;
> +		lp_claim_parport_or_block(&lp_table[minor]);
> +		status = r_str(minor);
> +		lp_release_parport(&lp_table[minor]);
> +		mutex_unlock(&lp_table[minor].port_mutex);
> +
> +		if (copy_to_user(argp, &status, sizeof(int)))
> +			return -EFAULT;
> +		break;
> +	case LPRESET:
> +		lp_reset(minor);
> +		break;
>  #ifdef LP_STATS
> -		case LPGETSTATS:
> -			if (copy_to_user(argp, &LP_STAT(minor),
> -					sizeof(struct lp_stats)))
> -				return -EFAULT;
> -			if (capable(CAP_SYS_ADMIN))
> -				memset(&LP_STAT(minor), 0,
> -						sizeof(struct lp_stats));
> -			break;
> +	case LPGETSTATS:
> +		if (copy_to_user(argp, &LP_STAT(minor),
> +				sizeof(struct lp_stats)))
> +			return -EFAULT;
> +		if (capable(CAP_SYS_ADMIN))
> +			memset(&LP_STAT(minor), 0,
> +					sizeof(struct lp_stats));
> +		break;
>  #endif
> -		case LPGETFLAGS:
> -			status = LP_F(minor);
> -			if (copy_to_user(argp, &status, sizeof(int)))
> -				return -EFAULT;
> -			break;
> +	case LPGETFLAGS:
> +		status = LP_F(minor);
> +		if (copy_to_user(argp, &status, sizeof(int)))
> +			return -EFAULT;
> +		break;
>  
> -		default:
> -			retval = -EINVAL;
> +	default:
> +		retval = -EINVAL;
>  	}
>  	return retval;
>  }
> @@ -688,9 +695,9 @@ static int lp_set_timeout(unsigned int minor, s64 tv_sec, long tv_usec)
>  		to_jiffies += tv_sec * (long) HZ;
>  	}
>  
> -	if (to_jiffies <= 0) {
> +	if (to_jiffies <= 0)
>  		return -EINVAL;
> -	}
> +
>  	lp_table[minor].timeout = to_jiffies;
>  	return 0;
>  }
> @@ -802,15 +809,17 @@ static const struct file_operations lp_fops = {
>  
>  #define CONSOLE_LP 0
>  
> -/* If the printer is out of paper, we can either lose the messages or
> +/*
> + * If the printer is out of paper, we can either lose the messages or
>   * stall until the printer is happy again.  Define CONSOLE_LP_STRICT
> - * non-zero to get the latter behaviour. */
> + * non-zero to get the latter behaviour.
> + */
>  #define CONSOLE_LP_STRICT 1
>  
>  /* The console must be locked when we get here. */
>  
>  static void lp_console_write(struct console *co, const char *s,
> -			     unsigned count)
> +			     unsigned int count)
>  {
>  	struct pardevice *dev = lp_table[CONSOLE_LP].dev;
>  	struct parport *port = dev->port;
> @@ -829,6 +838,7 @@ static void lp_console_write(struct console *co, const char *s,
>  		/* Write the data, converting LF->CRLF as we go. */
>  		ssize_t canwrite = count;
>  		char *lf = memchr(s, '\n', count);
> +
>  		if (lf)
>  			canwrite = lf - s;
>  
> @@ -891,24 +901,28 @@ static int __init lp_setup(char *str)
>  			/* disable driver on "lp=" or "lp=0" */
>  			parport_nr[0] = LP_PARPORT_OFF;
>  		} else {
> -			printk(KERN_WARNING "warning: 'lp=0x%x' is deprecated, ignored\n", x);
> +			pr_warn("warning: 'lp=0x%x' is deprecated, ignored\n", x);
>  			return 0;
>  		}
>  	} else if (!strncmp(str, "parport", 7)) {
> -		int n = simple_strtoul(str+7, NULL, 10);
> +		int err;
> +		unsigned long n = 0;
> +
> +		err = kstrtoul(str+7, 10, &n);
> +		if (err)
> +			return err;
> +
>  		if (parport_ptr < LP_NO)
>  			parport_nr[parport_ptr++] = n;
>  		else
> -			printk(KERN_INFO "lp: too many ports, %s ignored.\n",
> -			       str);
> +			pr_info("lp: too many ports, %s ignored.\n", str);
>  	} else if (!strcmp(str, "auto")) {
>  		parport_nr[0] = LP_PARPORT_AUTO;
>  	} else if (!strcmp(str, "none")) {
>  		if (parport_ptr < LP_NO)
>  			parport_nr[parport_ptr++] = LP_PARPORT_NONE;
>  		else
> -			printk(KERN_INFO "lp: too many ports, %s ignored.\n",
> -			       str);
> +			pr_info("lp: too many ports, %s ignored.\n", str);
>  	} else if (!strcmp(str, "reset")) {
>  		reset = true;
>  	}
> @@ -935,7 +949,7 @@ static int lp_register(int nr, struct parport *port)
>  	device_create(lp_class, port->dev, MKDEV(LP_MAJOR, nr), NULL,
>  		      "lp%d", nr);
>  
> -	printk(KERN_INFO "lp%d: using %s (%s).\n", nr, port->name,
> +	pr_info("lp%d: using %s (%s).\n", nr, port->name,
>  	       (port->irq == PARPORT_IRQ_NONE)?"polling":"interrupt-driven");
>  
>  #ifdef CONFIG_LP_CONSOLE
> @@ -943,9 +957,9 @@ static int lp_register(int nr, struct parport *port)
>  		if (port->modes & PARPORT_MODE_SAFEININT) {
>  			register_console(&lpcons);
>  			console_registered = port;
> -			printk(KERN_INFO "lp%d: console ready\n", CONSOLE_LP);
> +			pr_info("lp%d: console ready\n", CONSOLE_LP);
>  		} else
> -			printk(KERN_ERR "lp%d: cannot run console on %s\n",
> +			pr_err("lp%d: cannot run console on %s\n",
>  			       CONSOLE_LP, port->name);
>  	}
>  #endif
> @@ -965,7 +979,7 @@ static void lp_attach(struct parport *port)
>  		    port->probe_info[0].class != PARPORT_CLASS_PRINTER)
>  			return;
>  		if (lp_count == LP_NO) {
> -			printk(KERN_INFO "lp: ignoring parallel port (max. %d)\n",LP_NO);
> +			pr_info("lp: ignoring parallel port (max. %d)\n", LP_NO);
>  			return;
>  		}
>  		for (i = 0; i < LP_NO; i++)
> @@ -1045,7 +1059,7 @@ static int __init lp_init(void)
>  	}
>  
>  	if (register_chrdev(LP_MAJOR, "lp", &lp_fops)) {
> -		printk(KERN_ERR "lp: unable to get major %d\n", LP_MAJOR);
> +		pr_err("lp: unable to get major %d\n", LP_MAJOR);
>  		return -EIO;
>  	}
>  
> @@ -1056,16 +1070,16 @@ static int __init lp_init(void)
>  	}
>  
>  	if (parport_register_driver(&lp_driver)) {
> -		printk(KERN_ERR "lp: unable to register with parport\n");
> +		pr_err("lp: unable to register with parport\n");
>  		err = -EIO;
>  		goto out_class;
>  	}
>  
>  	if (!lp_count) {
> -		printk(KERN_INFO "lp: driver loaded but no devices found\n");
> +		pr_info("lp: driver loaded but no devices found\n");
>  #ifndef CONFIG_PARPORT_1284
>  		if (parport_nr[0] == LP_PARPORT_AUTO)
> -			printk(KERN_INFO "lp: (is IEEE 1284 support enabled?)\n");
> +			pr_info("lp: (is IEEE 1284 support enabled?)\n");
>  #endif
>  	}
>  
> @@ -1086,16 +1100,19 @@ static int __init lp_init_module(void)
>  			parport_nr[0] = LP_PARPORT_AUTO;
>  		else {
>  			int n;
> +
>  			for (n = 0; n < LP_NO && parport[n]; n++) {
>  				if (!strncmp(parport[n], "none", 4))
>  					parport_nr[n] = LP_PARPORT_NONE;
>  				else {
>  					char *ep;
> +					/* Don't update to kstrtoul, ep needs to be updated */
>  					unsigned long r = simple_strtoul(parport[n], &ep, 0);
> +
>  					if (ep != parport[n])
>  						parport_nr[n] = r;
>  					else {
> -						printk(KERN_ERR "lp: bad port specifier `%s'\n", parport[n]);
> +						pr_err("lp: bad port specifier `%s'\n", parport[n]);
>  						return -ENODEV;
>  					}
>  				}
> -- 
> 2.36.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH 4/5] audit: remove redundant data_len check

[Shreenidhi Shedi]

Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
---
 kernel/audit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 7690c29d4ee4..0749211d5552 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1390,7 +1390,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 						 str);
 			} else {
 				audit_log_format(ab, " data=");
-				if (data_len > 0 && str[data_len - 1] == '\0')
+				if (str[data_len - 1] == '\0')
 					data_len--;
 				audit_log_n_untrustedstring(ab, str, data_len);
 			}
-- 
2.36.1

Re: [PATCH 4/5] audit: remove redundant data_len check

[Greg KH]

On Sun, May 15, 2022 at 02:44:54AM +0530, Shreenidhi Shedi wrote:
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
> ---
>  kernel/audit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 7690c29d4ee4..0749211d5552 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1390,7 +1390,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  						 str);
>  			} else {
>  				audit_log_format(ab, " data=");
> -				if (data_len > 0 && str[data_len - 1] == '\0')
> +				if (str[data_len - 1] == '\0')
>  					data_len--;
>  				audit_log_n_untrustedstring(ab, str, data_len);
>  			}
> -- 
> 2.36.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH 4/5] audit: remove redundant data_len check

[Paul Moore]

On Sat, May 14, 2022 at 5:15 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
> ---
>  kernel/audit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

As a FYI for the non-audit folks on the To/CC line, patches 4/5 and
5/5 were resent to just the audit relevant folks so I'll follow-up
there.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 7690c29d4ee4..0749211d5552 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1390,7 +1390,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>                                                  str);
>                         } else {
>                                 audit_log_format(ab, " data=");
> -                               if (data_len > 0 && str[data_len - 1] == '\0')
> +                               if (str[data_len - 1] == '\0')
>                                         data_len--;
>                                 audit_log_n_untrustedstring(ab, str, data_len);
>                         }
> --
> 2.36.1

-- 
paul-moore.com

[PATCH 5/5] audit: fix most of the checkspec.pl warnnings & errors

[Shreenidhi Shedi]

Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
---
 kernel/audit.c | 205 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 127 insertions(+), 78 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 0749211d5552..b3a5f65ee357 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -26,7 +26,7 @@
  *	     information that must be passed to user-space.
  *
  * Audit userspace, documentation, tests, and bug/issue trackers:
- * 	https://github.com/linux-audit
+ *	https://github.com/linux-audit
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -62,16 +62,19 @@
 
 #include "audit.h"
 
-/* No auditing will take place until audit_initialized == AUDIT_INITIALIZED.
- * (Initialization happens after skb_init is called.) */
+/*
+ * No auditing will take place until audit_initialized == AUDIT_INITIALIZED.
+ * (Initialization happens after skb_init is called.)
+ */
 #define AUDIT_DISABLED		-1
 #define AUDIT_UNINITIALIZED	0
 #define AUDIT_INITIALIZED	1
+
 static int	audit_initialized = AUDIT_UNINITIALIZED;
 
-u32		audit_enabled = AUDIT_OFF;
 bool		audit_ever_enabled = !!AUDIT_OFF;
 
+u32		audit_enabled = AUDIT_OFF;
 EXPORT_SYMBOL_GPL(audit_enabled);
 
 /* Default state when kernel boots without any parameters. */
@@ -111,13 +114,17 @@ struct auditd_connection {
 static struct auditd_connection __rcu *auditd_conn;
 static DEFINE_SPINLOCK(auditd_conn_lock);
 
-/* If audit_rate_limit is non-zero, limit the rate of sending audit records
+/*
+ * If audit_rate_limit is non-zero, limit the rate of sending audit records
  * to that number per second.  This prevents DoS attacks, but results in
- * audit records being dropped. */
+ * audit records being dropped.
+ */
 static u32	audit_rate_limit;
 
-/* Number of outstanding audit_buffers allowed.
- * When set to zero, this means unlimited. */
+/*
+ * Number of outstanding audit_buffers allowed.
+ * When set to zero, this means unlimited.
+ */
 static u32	audit_backlog_limit = 64;
 #define AUDIT_BACKLOG_WAIT_TIME (60 * HZ)
 static u32	audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;
@@ -127,13 +134,14 @@ static kuid_t		audit_sig_uid = INVALID_UID;
 static pid_t		audit_sig_pid = -1;
 static u32		audit_sig_sid;
 
-/* Records can be lost in several ways:
-   0) [suppressed in audit_alloc]
-   1) out of memory in audit_log_start [kmalloc of struct audit_buffer]
-   2) out of memory in audit_log_move [alloc_skb]
-   3) suppressed due to audit_rate_limit
-   4) suppressed due to audit_backlog_limit
-*/
+/*
+ * Records can be lost in several ways:
+ * 0) [suppressed in audit_alloc]
+ * 1) out of memory in audit_log_start [kmalloc of struct audit_buffer]
+ * 2) out of memory in audit_log_move [alloc_skb]
+ * 3) suppressed due to audit_rate_limit
+ * 4) suppressed due to audit_backlog_limit
+ */
 static atomic_t	audit_lost = ATOMIC_INIT(0);
 
 /* Monotonically increasing sum of time the kernel has spent
@@ -186,16 +194,20 @@ static struct audit_ctl_mutex {
 	void *owner;
 } audit_cmd_mutex;
 
-/* AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
+/*
+ * AUDIT_BUFSIZ is the size of the temporary buffer used for formatting
  * audit records.  Since printk uses a 1024 byte buffer, this buffer
- * should be at least that large. */
+ * should be at least that large.
+ */
 #define AUDIT_BUFSIZ 1024
 
-/* The audit_buffer is used when formatting an audit record.  The caller
+/*
+ * The audit_buffer is used when formatting an audit record.  The caller
  * locks briefly to get the record off the freelist or to allocate the
  * buffer, and locks briefly to send the buffer to the netlink layer or
  * to place it on a transmit queue.  Multiple audit_buffers can be in
- * use simultaneously. */
+ * use simultaneously.
+ */
 struct audit_buffer {
 	struct sk_buff       *skb;	/* formatted skb ready to send */
 	struct audit_context *ctx;	/* NULL or associated context */
@@ -305,8 +317,7 @@ void audit_panic(const char *message)
 	case AUDIT_FAIL_SILENT:
 		break;
 	case AUDIT_FAIL_PRINTK:
-		if (printk_ratelimit())
-			pr_err("%s\n", message);
+		pr_err_ratelimited("%s\n", message);
 		break;
 	case AUDIT_FAIL_PANIC:
 		panic("audit: %s\n", message);
@@ -316,15 +327,16 @@ void audit_panic(const char *message)
 
 static inline int audit_rate_check(void)
 {
-	static unsigned long	last_check = 0;
-	static int		messages   = 0;
+	static unsigned long	last_check;
+	static int		messages;
 	static DEFINE_SPINLOCK(lock);
 	unsigned long		flags;
 	unsigned long		now;
 	unsigned long		elapsed;
-	int			retval	   = 0;
+	int			retval = 0;
 
-	if (!audit_rate_limit) return 1;
+	if (!audit_rate_limit)
+		return 1;
 
 	spin_lock_irqsave(&lock, flags);
 	if (++messages < audit_rate_limit) {
@@ -350,10 +362,10 @@ static inline int audit_rate_check(void)
  * Emit at least 1 message per second, even if audit_rate_check is
  * throttling.
  * Always increment the lost messages counter.
-*/
+ */
 void audit_log_lost(const char *message)
 {
-	static unsigned long	last_msg = 0;
+	static unsigned long	last_msg;
 	static DEFINE_SPINLOCK(lock);
 	unsigned long		flags;
 	unsigned long		now;
@@ -374,11 +386,10 @@ void audit_log_lost(const char *message)
 	}
 
 	if (print) {
-		if (printk_ratelimit())
-			pr_warn("audit_lost=%u audit_rate_limit=%u audit_backlog_limit=%u\n",
-				atomic_read(&audit_lost),
-				audit_rate_limit,
-				audit_backlog_limit);
+		pr_warn_ratelimited("audit_lost=%u audit_rate_limit=%u audit_backlog_limit=%u\n",
+			atomic_read(&audit_lost),
+			audit_rate_limit,
+			audit_backlog_limit);
 		audit_panic(message);
 	}
 }
@@ -447,6 +458,7 @@ static int audit_set_backlog_wait_time(u32 timeout)
 static int audit_set_enabled(u32 state)
 {
 	int rc;
+
 	if (state > AUDIT_LOCKED)
 		return -EINVAL;
 
@@ -534,8 +546,8 @@ static void kauditd_printk_skb(struct sk_buff *skb)
 	struct nlmsghdr *nlh = nlmsg_hdr(skb);
 	char *data = nlmsg_data(nlh);
 
-	if (nlh->nlmsg_type != AUDIT_EOE && printk_ratelimit())
-		pr_notice("type=%d %s\n", nlh->nlmsg_type, data);
+	if (nlh->nlmsg_type != AUDIT_EOE)
+		pr_notice_ratelimited("type=%d %s\n", nlh->nlmsg_type, data);
 }
 
 /**
@@ -568,8 +580,10 @@ static void kauditd_rehold_skb(struct sk_buff *skb, __always_unused int error)
  */
 static void kauditd_hold_skb(struct sk_buff *skb, int error)
 {
-	/* at this point it is uncertain if we will ever send this to auditd so
-	 * try to send the message via printk before we go any further */
+	/*
+	 * at this point it is uncertain if we will ever send this to auditd so
+	 * try to send the message via printk before we go any further
+	 */
 	kauditd_printk_skb(skb);
 
 	/* can we just silently drop the message? */
@@ -659,8 +673,10 @@ static void auditd_reset(const struct auditd_connection *ac)
 	if (ac_old)
 		call_rcu(&ac_old->rcu, auditd_conn_free);
 
-	/* flush the retry queue to the hold queue, but don't touch the main
-	 * queue since we need to process that normally for multicast */
+	/*
+	 * flush the retry queue to the hold queue, but don't touch the main
+	 * queue since we need to process that normally for multicast
+	 */
 	while ((skb = skb_dequeue(&audit_retry_queue)))
 		kauditd_hold_skb(skb, -ECONNREFUSED);
 }
@@ -684,12 +700,14 @@ static int auditd_send_unicast_skb(struct sk_buff *skb)
 	struct sock *sk;
 	struct auditd_connection *ac;
 
-	/* NOTE: we can't call netlink_unicast while in the RCU section so
+	/*
+	 * NOTE: we can't call netlink_unicast while in the RCU section so
 	 *       take a reference to the network namespace and grab local
 	 *       copies of the namespace, the sock, and the portid; the
 	 *       namespace and sock aren't going to go away while we hold a
 	 *       reference and if the portid does become invalid after the RCU
-	 *       section netlink_unicast() should safely return an error */
+	 *       section netlink_unicast() should safely return an error
+	 */
 
 	rcu_read_lock();
 	ac = rcu_dereference(auditd_conn);
@@ -743,8 +761,10 @@ static int kauditd_send_queue(struct sock *sk, u32 portid,
 	struct sk_buff *skb_tail;
 	unsigned int failed = 0;
 
-	/* NOTE: kauditd_thread takes care of all our locking, we just use
-	 *       the netlink info passed to us (e.g. sk and portid) */
+	/*
+	 * NOTE: kauditd_thread takes care of all our locking, we just use
+	 *       the netlink info passed to us (e.g. sk and portid)
+	 */
 
 	skb_tail = skb_peek_tail(queue);
 	while ((skb != skb_tail) && (skb = skb_dequeue(queue))) {
@@ -801,8 +821,10 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb)
 	struct sock *sock = audit_get_sk(&init_net);
 	struct nlmsghdr *nlh;
 
-	/* NOTE: we are not taking an additional reference for init_net since
-	 *       we don't have to worry about it going away */
+	/*
+	 * NOTE: we are not taking an additional reference for init_net since
+	 *       we don't have to worry about it going away
+	 */
 
 	if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG))
 		return;
@@ -875,10 +897,12 @@ static int kauditd_thread(void *dummy)
 		}
 
 main_queue:
-		/* process the main queue - do the multicast send and attempt
+		/*
+		 * process the main queue - do the multicast send and attempt
 		 * unicast, dump failed record sends to the retry queue; if
 		 * sk == NULL due to previous failures we will just do the
-		 * multicast send and move the record to the hold queue */
+		 * multicast send and move the record to the hold queue
+		 */
 		rc = kauditd_send_queue(sk, portid, &audit_queue, 1,
 					kauditd_send_multicast_skb,
 					(sk ?
@@ -896,10 +920,12 @@ static int kauditd_thread(void *dummy)
 		/* we have processed all the queues so wake everyone */
 		wake_up(&audit_backlog_wait);
 
-		/* NOTE: we want to wake up if there is anything on the queue,
+		/*
+		 * NOTE: we want to wake up if there is anything on the queue,
 		 *       regardless of if an auditd is connected, as we need to
 		 *       do the multicast send and rotate records from the
-		 *       main queue to the retry/hold queues */
+		 *       main queue to the retry/hold queues
+		 */
 		wait_event_freezable(kauditd_wait,
 				     (skb_queue_len(&audit_queue) ? 1 : 0));
 	}
@@ -969,8 +995,10 @@ static int audit_send_reply_thread(void *arg)
 	audit_ctl_lock();
 	audit_ctl_unlock();
 
-	/* Ignore failure. It'll only happen if the sender goes away,
-	   because our timeout is set to infinite. */
+	/*
+	 * Ignore failure. It'll only happen if the sender goes away,
+	 * because our timeout is set to infinite.
+	 */
 	netlink_unicast(audit_get_sk(reply->net), reply->skb, reply->portid, 0);
 	reply->skb = NULL;
 	audit_free_reply(reply);
@@ -1054,8 +1082,10 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 	case AUDIT_TTY_SET:
 	case AUDIT_TRIM:
 	case AUDIT_MAKE_EQUIV:
-		/* Only support auditd and auditctl in initial pid namespace
-		 * for now. */
+		/*
+		 * Only support auditd and auditctl in initial pid namespace
+		 * for now.
+		 */
 		if (task_active_pid_ns(current) != &init_pid_ns)
 			return -EPERM;
 
@@ -1226,11 +1256,14 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	switch (msg_type) {
 	case AUDIT_GET: {
 		struct audit_status	s;
+
 		memset(&s, 0, sizeof(s));
 		s.enabled		   = audit_enabled;
 		s.failure		   = audit_failure;
-		/* NOTE: use pid_vnr() so the PID is relative to the current
-		 *       namespace */
+		/*
+		 * NOTE: use pid_vnr() so the PID is relative to the current
+		 *       namespace
+		 */
 		s.pid			   = auditd_pid_vnr();
 		s.rate_limit		   = audit_rate_limit;
 		s.backlog_limit		   = audit_backlog_limit;
@@ -1244,6 +1277,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	}
 	case AUDIT_SET: {
 		struct audit_status	s;
+
 		memset(&s, 0, sizeof(s));
 		/* guard against past and future API changes */
 		memcpy(&s, data, min_t(size_t, sizeof(s), data_len));
@@ -1258,18 +1292,22 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 				return err;
 		}
 		if (s.mask & AUDIT_STATUS_PID) {
-			/* NOTE: we are using the vnr PID functions below
+			/*
+			 * NOTE: we are using the vnr PID functions below
 			 *       because the s.pid value is relative to the
 			 *       namespace of the caller; at present this
 			 *       doesn't matter much since you can really only
 			 *       run auditd from the initial pid namespace, but
-			 *       something to keep in mind if this changes */
+			 *       something to keep in mind if this changes
+			 */
 			pid_t new_pid = s.pid;
 			pid_t auditd_pid;
 			struct pid *req_pid = task_tgid(current);
 
-			/* Sanity check - PID values must match. Setting
-			 * pid to 0 is how auditd ends auditing. */
+			/*
+			 * Sanity check - PID values must match. Setting
+			 * pid to 0 is how auditd ends auditing.
+			 */
 			if (new_pid && (new_pid != pid_vnr(req_pid)))
 				return -EINVAL;
 
@@ -1655,11 +1693,13 @@ static void __net_exit audit_net_exit(struct net *net)
 {
 	struct audit_net *aunet = net_generic(net, audit_net_id);
 
-	/* NOTE: you would think that we would want to check the auditd
+	/*
+	 * NOTE: you would think that we would want to check the auditd
 	 * connection and potentially reset it here if it lives in this
 	 * namespace, but since the auditd connection tracking struct holds a
 	 * reference to this namespace (see auditd_set()) we are only ever
-	 * going to get here after that connection has been released */
+	 * going to get here after that connection has been released
+	 */
 
 	netlink_kernel_release(aunet->sk);
 }
@@ -1702,6 +1742,7 @@ static int __init audit_init(void)
 	kauditd_task = kthread_run(kauditd_thread, NULL, "kauditd");
 	if (IS_ERR(kauditd_task)) {
 		int err = PTR_ERR(kauditd_task);
+
 		panic("audit: failed to start the kauditd thread (%d)\n", err);
 	}
 
@@ -1741,21 +1782,23 @@ static int __init audit_enable(char *str)
 }
 __setup("audit=", audit_enable);
 
-/* Process kernel command-line parameter at boot time.
- * audit_backlog_limit=<n> */
+/*
+ * Process kernel command-line parameter at boot time.
+ * audit_backlog_limit=<n>
+ */
 static int __init audit_backlog_limit_set(char *str)
 {
 	u32 audit_backlog_limit_arg;
 
 	pr_info("audit_backlog_limit: ");
 	if (kstrtouint(str, 0, &audit_backlog_limit_arg)) {
-		pr_cont("using default of %u, unable to parse %s\n",
+		pr_info("using default of %u, unable to parse %s\n",
 			audit_backlog_limit, str);
 		return 1;
 	}
 
 	audit_backlog_limit = audit_backlog_limit_arg;
-	pr_cont("%d\n", audit_backlog_limit);
+	pr_info("%d\n", audit_backlog_limit);
 
 	return 1;
 }
@@ -1873,8 +1916,10 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 			/* wake kauditd to try and flush the queue */
 			wake_up_interruptible(&kauditd_wait);
 
-			/* sleep if we are allowed and we haven't exhausted our
-			 * backlog wait limit */
+			/*
+			 * sleep if we are allowed and we haven't exhausted our
+			 * backlog wait limit
+			 */
 			if (gfpflags_allow_blocking(gfp_mask) && (stime > 0)) {
 				long rtime = stime;
 
@@ -1887,8 +1932,8 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 				atomic_add(rtime - stime, &audit_backlog_wait_time_actual);
 				remove_wait_queue(&audit_backlog_wait, &wait);
 			} else {
-				if (audit_rate_check() && printk_ratelimit())
-					pr_warn("audit_backlog=%d > audit_backlog_limit=%d\n",
+				if (audit_rate_check())
+					pr_warn_ratelimited("audit_backlog=%d > audit_backlog_limit=%d\n",
 						skb_queue_len(&audit_queue),
 						audit_backlog_limit);
 				audit_log_lost("backlog limit exceeded");
@@ -1912,6 +1957,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 
 	return ab;
 }
+EXPORT_SYMBOL(audit_log_start);
 
 /**
  * audit_expand - expand skb in the audit buffer
@@ -1964,11 +2010,13 @@ static void audit_log_vformat(struct audit_buffer *ab, const char *fmt,
 	va_copy(args2, args);
 	len = vsnprintf(skb_tail_pointer(skb), avail, fmt, args);
 	if (len >= avail) {
-		/* The printk buffer is 1024 bytes long, so if we get
+		/*
+		 * The printk buffer is 1024 bytes long, so if we get
 		 * here and AUDIT_BUFSIZ is at least 1024, then we can
-		 * log everything that printk could have logged. */
+		 * log everything that printk could have logged.
+		 */
 		avail = audit_expand(ab,
-			max_t(unsigned, AUDIT_BUFSIZ, 1+len-avail));
+			max_t(unsigned int, AUDIT_BUFSIZ, 1+len-avail));
 		if (!avail)
 			goto out_va_end;
 		len = vsnprintf(skb_tail_pointer(skb), avail, fmt, args2);
@@ -1999,6 +2047,7 @@ void audit_log_format(struct audit_buffer *ab, const char *fmt, ...)
 	audit_log_vformat(ab, fmt, args);
 	va_end(args);
 }
+EXPORT_SYMBOL(audit_log_format);
 
 /**
  * audit_log_n_hex - convert a buffer to hex and append it to the audit skb
@@ -2080,6 +2129,7 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string,
 bool audit_string_contains_control(const char *string, size_t len)
 {
 	const unsigned char *p;
+
 	for (p = string; p < (const unsigned char *)string + len; p++) {
 		if (*p == '"' || *p < 0x21 || *p > 0x7e)
 			return true;
@@ -2167,7 +2217,7 @@ void audit_log_key(struct audit_buffer *ab, char *key)
 int audit_log_task_context(struct audit_buffer *ab)
 {
 	char *ctx = NULL;
-	unsigned len;
+	unsigned int len;
 	int error;
 	u32 sid;
 
@@ -2419,8 +2469,10 @@ void audit_log_end(struct audit_buffer *ab)
 		skb = ab->skb;
 		ab->skb = NULL;
 
-		/* setup the netlink header, see the comments in
-		 * kauditd_send_multicast_skb() for length quirks */
+		/*
+		 * setup the netlink header, see the comments in
+		 * kauditd_send_multicast_skb() for length quirks
+		 */
 		nlh = nlmsg_hdr(skb);
 		nlh->nlmsg_len = skb->len - NLMSG_HDRLEN;
 
@@ -2432,6 +2484,7 @@ void audit_log_end(struct audit_buffer *ab)
 
 	audit_buffer_free(ab);
 }
+EXPORT_SYMBOL(audit_log_end);
 
 /**
  * audit_log - Log an audit record
@@ -2459,8 +2512,4 @@ void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
 		audit_log_end(ab);
 	}
 }
-
-EXPORT_SYMBOL(audit_log_start);
-EXPORT_SYMBOL(audit_log_end);
-EXPORT_SYMBOL(audit_log_format);
 EXPORT_SYMBOL(audit_log);
-- 
2.36.1

Re: [PATCH 5/5] audit: fix most of the checkspec.pl warnnings & errors

[Greg KH]

On Sun, May 15, 2022 at 02:44:55AM +0530, Shreenidhi Shedi wrote:
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH 1/5] char: lp: ensure that index has not exceeded LP_NO

[Greg KH]

On Sun, May 15, 2022 at 02:44:51AM +0530, Shreenidhi Shedi wrote:
> Signed-off-by: Shreenidhi Shedi <sshedi@vmware.com>
> ---
>  drivers/char/lp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/lp.c b/drivers/char/lp.c
> index 0e22e3b0a04e..d474d02b6c7f 100644
> --- a/drivers/char/lp.c
> +++ b/drivers/char/lp.c
> @@ -972,7 +972,7 @@ static void lp_attach(struct parport *port)
>  			if (port_num[i] == -1)
>  				break;
>  
> -		if (!lp_register(i, port))
> +		if (i < LP_NO && !lp_register(i, port))
>  			lp_count++;
>  		break;
>  
> -- 
> 2.36.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot