[PATCH 0/5] scsi: Replace {v}snprintf() variants with safer alternatives

[PATCH 0/5] scsi: Replace {v}snprintf() variants with safer alternatives

[Lee Jones]

For a far better description of the problem than I could author, see
Jon's write-up on LWN [1] and/or Alex's on the Kernel Self Protection
Project [1].

[0] https://lwn.net/Articles/69419/
[1] https://github.com/KSPP/linux/issues/105

Lee Jones (5):
  scsi: 3w-9xxx: Remove snprintf() from sysfs call-backs and replace
    with sysfs_emit()
  scsi: 3w-sas: Remove snprintf() from sysfs call-backs and replace with
    sysfs_emit()
  scsi: 3w-xxxx: Remove snprintf() from sysfs call-backs and replace
    with sysfs_emit()
  scsi: 53c700: Remove snprintf() from sysfs call-backs and replace with
    sysfs_emit()
  scsi: aacraid: aachba: Replace snprintf() with the safer scnprintf()
    variant

 drivers/scsi/3w-9xxx.c        |  44 +++++++-------
 drivers/scsi/3w-sas.c         |  36 ++++++------
 drivers/scsi/3w-xxxx.c        |  46 +++++++--------
 drivers/scsi/53c700.c         | 104 +++++++++++++++++-----------------
 drivers/scsi/aacraid/aachba.c |   6 +-
 5 files changed, 118 insertions(+), 118 deletions(-)

Cc: Adam Radford <aradford@gmail.com>
Cc: Adaptec OEM Raid Solutions <aacraid@microsemi.com>
Cc: Andre Hedrick <andre@suse.com>
Cc: de Melo <acme@conectiva.com.br>
Cc: Joel Jacobson <linux@3ware.com>
Cc: "PMC-Sierra, Inc" <aacraid@pmc-sierra.com>
Cc: Richard Hirst <rhirst@linuxcare.com>
-- 
2.43.0.275.g3460e3d667-goog

[PATCH 1/5] scsi: 3w-9xxx: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()

[Lee Jones]

Since snprintf() has the documented, but still rather strange trait of
returning the length of the data that *would have been* written to the
array if space were available, rather than the arguably more useful
length of data *actually* written, it is usually considered wise to use
something else instead in order to avoid confusion.

In the case of sysfs call-backs, new wrappers exist that do just that.

Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Adam Radford <aradford@gmail.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/scsi/3w-9xxx.c | 44 +++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index f925f8664c2c1..6fb61c88ea119 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -161,28 +161,28 @@ static ssize_t twa_show_stats(struct device *dev,
 	ssize_t len;
 
 	spin_lock_irqsave(tw_dev->host->host_lock, flags);
-	len = snprintf(buf, PAGE_SIZE, "3w-9xxx Driver version: %s\n"
-		       "Current commands posted:   %4d\n"
-		       "Max commands posted:       %4d\n"
-		       "Current pending commands:  %4d\n"
-		       "Max pending commands:      %4d\n"
-		       "Last sgl length:           %4d\n"
-		       "Max sgl length:            %4d\n"
-		       "Last sector count:         %4d\n"
-		       "Max sector count:          %4d\n"
-		       "SCSI Host Resets:          %4d\n"
-		       "AEN's:                     %4d\n",
-		       TW_DRIVER_VERSION,
-		       tw_dev->posted_request_count,
-		       tw_dev->max_posted_request_count,
-		       tw_dev->pending_request_count,
-		       tw_dev->max_pending_request_count,
-		       tw_dev->sgl_entries,
-		       tw_dev->max_sgl_entries,
-		       tw_dev->sector_count,
-		       tw_dev->max_sector_count,
-		       tw_dev->num_resets,
-		       tw_dev->aen_count);
+	len = sysfs_emit(buf, "3w-9xxx Driver version: %s\n"
+			 "Current commands posted:   %4d\n"
+			 "Max commands posted:       %4d\n"
+			 "Current pending commands:  %4d\n"
+			 "Max pending commands:      %4d\n"
+			 "Last sgl length:           %4d\n"
+			 "Max sgl length:            %4d\n"
+			 "Last sector count:         %4d\n"
+			 "Max sector count:          %4d\n"
+			 "SCSI Host Resets:          %4d\n"
+			 "AEN's:                     %4d\n",
+			 TW_DRIVER_VERSION,
+			 tw_dev->posted_request_count,
+			 tw_dev->max_posted_request_count,
+			 tw_dev->pending_request_count,
+			 tw_dev->max_pending_request_count,
+			 tw_dev->sgl_entries,
+			 tw_dev->max_sgl_entries,
+			 tw_dev->sector_count,
+			 tw_dev->max_sector_count,
+			 tw_dev->num_resets,
+			 tw_dev->aen_count);
 	spin_unlock_irqrestore(tw_dev->host->host_lock, flags);
 	return len;
 } /* End twa_show_stats() */
-- 
2.43.0.275.g3460e3d667-goog

[PATCH 2/5] scsi: 3w-sas: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()

[Lee Jones]

Since snprintf() has the documented, but still rather strange trait of
returning the length of the data that *would have been* written to the
array if space were available, rather than the arguably more useful
length of data *actually* written, it is usually considered wise to use
something else instead in order to avoid confusion.

In the case of sysfs call-backs, new wrappers exist that do just that.

Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Adam Radford <aradford@gmail.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/scsi/3w-sas.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index 9bdb75dfdcd7c..caa6713a62a44 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -166,24 +166,24 @@ static ssize_t twl_show_stats(struct device *dev,
 	ssize_t len;
 
 	spin_lock_irqsave(tw_dev->host->host_lock, flags);
-	len = snprintf(buf, PAGE_SIZE, "3w-sas Driver version: %s\n"
-		       "Current commands posted:   %4d\n"
-		       "Max commands posted:       %4d\n"
-		       "Last sgl length:           %4d\n"
-		       "Max sgl length:            %4d\n"
-		       "Last sector count:         %4d\n"
-		       "Max sector count:          %4d\n"
-		       "SCSI Host Resets:          %4d\n"
-		       "AEN's:                     %4d\n",
-		       TW_DRIVER_VERSION,
-		       tw_dev->posted_request_count,
-		       tw_dev->max_posted_request_count,
-		       tw_dev->sgl_entries,
-		       tw_dev->max_sgl_entries,
-		       tw_dev->sector_count,
-		       tw_dev->max_sector_count,
-		       tw_dev->num_resets,
-		       tw_dev->aen_count);
+	len = sysfs_emit(buf, "3w-sas Driver version: %s\n"
+			 "Current commands posted:   %4d\n"
+			 "Max commands posted:       %4d\n"
+			 "Last sgl length:           %4d\n"
+			 "Max sgl length:            %4d\n"
+			 "Last sector count:         %4d\n"
+			 "Max sector count:          %4d\n"
+			 "SCSI Host Resets:          %4d\n"
+			 "AEN's:                     %4d\n",
+			 TW_DRIVER_VERSION,
+			 tw_dev->posted_request_count,
+			 tw_dev->max_posted_request_count,
+			 tw_dev->sgl_entries,
+			 tw_dev->max_sgl_entries,
+			 tw_dev->sector_count,
+			 tw_dev->max_sector_count,
+			 tw_dev->num_resets,
+			 tw_dev->aen_count);
 	spin_unlock_irqrestore(tw_dev->host->host_lock, flags);
 	return len;
 } /* End twl_show_stats() */
-- 
2.43.0.275.g3460e3d667-goog

[PATCH 3/5] scsi: 3w-xxxx: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()

[Lee Jones]

Since snprintf() has the documented, but still rather strange trait of
returning the length of the data that *would have been* written to the
array if space were available, rather than the arguably more useful
length of data *actually* written, it is usually considered wise to use
something else instead in order to avoid confusion.

In the case of sysfs call-backs, new wrappers exist that do just that.

Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Adam Radford <aradford@gmail.com>
Cc: Joel Jacobson <linux@3ware.com>
Cc: de Melo <acme@conectiva.com.br>
Cc: Andre Hedrick <andre@suse.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/scsi/3w-xxxx.c | 46 +++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
index f39c9ec2e7810..13ddc97f5a623 100644
--- a/drivers/scsi/3w-xxxx.c
+++ b/drivers/scsi/3w-xxxx.c
@@ -496,28 +496,28 @@ static ssize_t tw_show_stats(struct device *dev, struct device_attribute *attr,
 	ssize_t len;
 
 	spin_lock_irqsave(tw_dev->host->host_lock, flags);
-	len = snprintf(buf, PAGE_SIZE, "3w-xxxx Driver version: %s\n"
-		       "Current commands posted:   %4d\n"
-		       "Max commands posted:       %4d\n"
-		       "Current pending commands:  %4d\n"
-		       "Max pending commands:      %4d\n"
-		       "Last sgl length:           %4d\n"
-		       "Max sgl length:            %4d\n"
-		       "Last sector count:         %4d\n"
-		       "Max sector count:          %4d\n"
-		       "SCSI Host Resets:          %4d\n"
-		       "AEN's:                     %4d\n", 
-		       TW_DRIVER_VERSION,
-		       tw_dev->posted_request_count,
-		       tw_dev->max_posted_request_count,
-		       tw_dev->pending_request_count,
-		       tw_dev->max_pending_request_count,
-		       tw_dev->sgl_entries,
-		       tw_dev->max_sgl_entries,
-		       tw_dev->sector_count,
-		       tw_dev->max_sector_count,
-		       tw_dev->num_resets,
-		       tw_dev->aen_count);
+	len = sysfs_emit(buf, "3w-xxxx Driver version: %s\n"
+			 "Current commands posted:   %4d\n"
+			 "Max commands posted:       %4d\n"
+			 "Current pending commands:  %4d\n"
+			 "Max pending commands:      %4d\n"
+			 "Last sgl length:           %4d\n"
+			 "Max sgl length:            %4d\n"
+			 "Last sector count:         %4d\n"
+			 "Max sector count:          %4d\n"
+			 "SCSI Host Resets:          %4d\n"
+			 "AEN's:                     %4d\n",
+			 TW_DRIVER_VERSION,
+			 tw_dev->posted_request_count,
+			 tw_dev->max_posted_request_count,
+			 tw_dev->pending_request_count,
+			 tw_dev->max_pending_request_count,
+			 tw_dev->sgl_entries,
+			 tw_dev->max_sgl_entries,
+			 tw_dev->sector_count,
+			 tw_dev->max_sector_count,
+			 tw_dev->num_resets,
+			 tw_dev->aen_count);
 	spin_unlock_irqrestore(tw_dev->host->host_lock, flags);
 	return len;
 } /* End tw_show_stats() */
@@ -1099,7 +1099,7 @@ static int tw_initconnection(TW_Device_Extension *tw_dev, int message_credits)
 	command_packet->request_id = request_id;
 	command_packet->status = 0x0;
 	command_packet->flags = 0x0;
-	command_packet->byte6.message_credits = message_credits; 
+	command_packet->byte6.message_credits = message_credits;
 	command_packet->byte8.init_connection.response_queue_pointer = 0x0;
 	command_que_value = tw_dev->command_packet_physical_address[request_id];
 
-- 
2.43.0.275.g3460e3d667-goog

[PATCH 4/5] scsi: 53c700: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()

[Lee Jones]

Since snprintf() has the documented, but still rather strange trait of
returning the length of the data that *would have been* written to the
array if space were available, rather than the arguably more useful
length of data *actually* written, it is usually considered wise to use
something else instead in order to avoid confusion.

In the case of sysfs call-backs, new wrappers exist that do just that.

Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Richard Hirst <rhirst@linuxcare.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/scsi/53c700.c | 104 +++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 857be0f3ae5b9..1aa933485719a 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -4,7 +4,7 @@
  *
  * Copyright (C) 2001 by James.Bottomley@HansenPartnership.com
 **-----------------------------------------------------------------------------
-**  
+**
 **
 **-----------------------------------------------------------------------------
  */
@@ -18,7 +18,7 @@
  *
  * The 700 is the lowliest of the line, it can only do async SCSI.
  * The 700-66 can at least do synchronous SCSI up to 10MHz.
- * 
+ *
  * The 700 chip has no host bus interface logic of its own.  However,
  * it is usually mapped to a location with well defined register
  * offsets.  Therefore, if you can determine the base address and the
@@ -61,7 +61,7 @@
  * consistent memory allocation.
  *
  * Version 2.5
- * 
+ *
  * More Compatibility changes for 710 (now actually works).  Enhanced
  * support for odd clock speeds which constrain SDTR negotiations.
  * correct cacheline separation for scsi messages and status for
@@ -70,7 +70,7 @@
  *
  * Version 2.4
  *
- * Added support for the 53c710 chip (in 53c700 emulation mode only---no 
+ * Added support for the 53c710 chip (in 53c700 emulation mode only---no
  * special 53c710 instructions or registers are used).
  *
  * Version 2.3
@@ -190,7 +190,7 @@ static char *NCR_700_condition[] = {
 	"DISCONNECT_MSG RECEIVED",
 	"MSG_OUT",
 	"DATA_IN",
-	
+
 };
 
 static char *NCR_700_fatal_messages[] = {
@@ -260,7 +260,7 @@ NCR_700_offset_period_to_sxfer(struct NCR_700_Host_Parameters *hostdata,
 static inline __u8
 NCR_700_get_SXFER(struct scsi_device *SDp)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SDp->host->hostdata[0];
 
 	return NCR_700_offset_period_to_sxfer(hostdata,
@@ -416,7 +416,7 @@ NCR_700_detect(struct scsi_host_template *tpnt,
 int
 NCR_700_release(struct Scsi_Host *host)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)host->hostdata[0];
 
 	if (hostdata->noncoherent)
@@ -449,7 +449,7 @@ NCR_700_identify(int can_disconnect, __u8 lun)
  * Inputs : host - SCSI host */
 static inline int
 NCR_700_data_residual (struct Scsi_Host *host) {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)host->hostdata[0];
 	int count, synchronous = 0;
 	unsigned int ddir;
@@ -461,16 +461,16 @@ NCR_700_data_residual (struct Scsi_Host *host) {
 		count = ((NCR_700_readb(host, DFIFO_REG) & 0x3f) -
 			 (NCR_700_readl(host, DBC_REG) & 0x3f)) & 0x3f;
 	}
-	
+
 	if(hostdata->fast)
 		synchronous = NCR_700_readb(host, SXFER_REG) & 0x0f;
-	
+
 	/* get the data direction */
 	ddir = NCR_700_readb(host, CTEST0_REG) & 0x01;
 
 	if (ddir) {
 		/* Receive */
-		if (synchronous) 
+		if (synchronous)
 			count += (NCR_700_readb(host, SSTAT2_REG) & 0xf0) >> 4;
 		else
 			if (NCR_700_readb(host, SSTAT1_REG) & SIDL_REG_FULL)
@@ -500,7 +500,7 @@ sbcl_to_string(__u8 sbcl)
 
 	ret[0]='\0';
 	for(i=0; i<8; i++) {
-		if((1<<i) & sbcl) 
+		if((1<<i) & sbcl)
 			strcat(ret, NCR_700_SBCL_bits[i]);
 	}
 	strcat(ret, NCR_700_SBCL_to_phase[sbcl & 0x07]);
@@ -533,7 +533,7 @@ find_empty_slot(struct NCR_700_Host_Parameters *hostdata)
 	if(slot->state != NCR_700_SLOT_FREE)
 		/* should panic! */
 		printk(KERN_ERR "BUSY SLOT ON FREE LIST!!!\n");
-		
+
 
 	hostdata->free_list = slot->ITL_forw;
 	slot->ITL_forw = NULL;
@@ -546,11 +546,11 @@ find_empty_slot(struct NCR_700_Host_Parameters *hostdata)
 	slot->state = NCR_700_SLOT_BUSY;
 	slot->flags = 0;
 	hostdata->command_slot_count++;
-	
+
 	return slot;
 }
 
-STATIC void 
+STATIC void
 free_slot(struct NCR_700_command_slot *slot,
 	  struct NCR_700_Host_Parameters *hostdata)
 {
@@ -560,7 +560,7 @@ free_slot(struct NCR_700_command_slot *slot,
 	if(slot->state == NCR_700_SLOT_FREE) {
 		printk(KERN_ERR "53c700: SLOT %p is FREE!!!\n", slot);
 	}
-	
+
 	slot->resume_offset = 0;
 	slot->cmnd = NULL;
 	slot->state = NCR_700_SLOT_FREE;
@@ -654,7 +654,7 @@ NCR_700_internal_bus_reset(struct Scsi_Host *host)
 STATIC void
 NCR_700_chip_setup(struct Scsi_Host *host)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)host->hostdata[0];
 	__u8 min_period;
 	__u8 min_xferp = (hostdata->chip710 ? NCR_710_MIN_XFERP : NCR_700_MIN_XFERP);
@@ -694,11 +694,11 @@ NCR_700_chip_setup(struct Scsi_Host *host)
 	} else {
 		NCR_700_writeb(BURST_LENGTH_8 | hostdata->dmode_extra,
 			       host, DMODE_700_REG);
-		NCR_700_writeb(hostdata->differential ? 
+		NCR_700_writeb(hostdata->differential ?
 			       DIFF : 0, host, CTEST7_REG);
 		if(hostdata->fast) {
 			/* this is for 700-66, does nothing on 700 */
-			NCR_700_writeb(LAST_DIS_ENBL | ENABLE_ACTIVE_NEGATION 
+			NCR_700_writeb(LAST_DIS_ENBL | ENABLE_ACTIVE_NEGATION
 				       | GENERATE_RECEIVE_PARITY, host,
 				       CTEST8_REG);
 		} else {
@@ -731,7 +731,7 @@ NCR_700_chip_setup(struct Scsi_Host *host)
 		NCR_700_writeb(ASYNC_DIV_3_0 | hostdata->dcntl_extra, host, DCNTL_REG);
 		hostdata->sync_clock = hostdata->clock*2;
 		hostdata->sync_clock /= 3;
-		
+
 	} else if(hostdata->clock > 37 && hostdata->clock <= 50) {
 		/* sync divider 1, async divider 2 */
 		DEBUG(("53c700: sync 1 async 2\n"));
@@ -764,7 +764,7 @@ NCR_700_chip_setup(struct Scsi_Host *host)
 STATIC void
 NCR_700_chip_reset(struct Scsi_Host *host)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)host->hostdata[0];
 	if(hostdata->chip710) {
 		NCR_700_writeb(SOFTWARE_RESET_710, host, ISTAT_REG);
@@ -774,7 +774,7 @@ NCR_700_chip_reset(struct Scsi_Host *host)
 	} else {
 		NCR_700_writeb(SOFTWARE_RESET, host, DCNTL_REG);
 		udelay(100);
-		
+
 		NCR_700_writeb(0, host, DCNTL_REG);
 	}
 
@@ -790,7 +790,7 @@ NCR_700_chip_reset(struct Scsi_Host *host)
  * ACK) so that the routine returns correctly to resume its activity
  * */
 STATIC __u32
-process_extended_message(struct Scsi_Host *host, 
+process_extended_message(struct Scsi_Host *host,
 			 struct NCR_700_Host_Parameters *hostdata,
 			 struct scsi_cmnd *SCp, __u32 dsp, __u32 dsps)
 {
@@ -816,15 +816,15 @@ process_extended_message(struct Scsi_Host *host,
 
 			spi_offset(starget) = offset;
 			spi_period(starget) = period;
-			
+
 			if(NCR_700_is_flag_set(SCp->device, NCR_700_DEV_PRINT_SYNC_NEGOTIATION)) {
 				spi_display_xfer_agreement(starget);
 				NCR_700_clear_flag(SCp->device, NCR_700_DEV_PRINT_SYNC_NEGOTIATION);
 			}
-			
+
 			NCR_700_set_flag(SCp->device, NCR_700_DEV_NEGOTIATED_SYNC);
 			NCR_700_clear_flag(SCp->device, NCR_700_DEV_BEGIN_SYNC_NEGOTIATION);
-			
+
 			NCR_700_writeb(NCR_700_get_SXFER(SCp->device),
 				       host, SXFER_REG);
 
@@ -841,7 +841,7 @@ process_extended_message(struct Scsi_Host *host,
 			resume_offset = hostdata->pScript + Ent_SendMessageWithATN;
 		}
 		break;
-	
+
 	case A_WDTR_MSG:
 		printk(KERN_INFO "scsi%d: (%d:%d), Unsolicited WDTR after CMD, Rejecting\n",
 		       host->host_no, pun, lun);
@@ -1121,7 +1121,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 
 			SCp = scsi_host_find_tag(SDp->host, hostdata->msgin[2]);
 			if(unlikely(SCp == NULL)) {
-				printk(KERN_ERR "scsi%d: (%d:%d) no saved request for tag %d\n", 
+				printk(KERN_ERR "scsi%d: (%d:%d) no saved request for tag %d\n",
 				       host->host_no, reselection_id, lun, hostdata->msgin[2]);
 				BUG();
 			}
@@ -1180,7 +1180,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 				       slot->cmnd->cmd_len);
 
 
-			
+
 		}
 	} else if(dsps == A_RESELECTED_DURING_SELECTION) {
 
@@ -1193,10 +1193,10 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 
 		__u8 reselection_id = NCR_700_readb(host, SFBR_REG);
 		struct NCR_700_command_slot *slot;
-		
+
 		/* Take out our own ID */
 		reselection_id &= ~(1<<host->this_id);
-		
+
 		/* I've never seen this happen, so keep this as a printk rather
 		 * than a debug */
 		printk(KERN_INFO "scsi%d: (%d:%d) RESELECTION DURING SELECTION, dsp=%08x[%04x] state=%d, count=%d\n",
@@ -1222,7 +1222,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 			slot->state = NCR_700_SLOT_QUEUED;
 		}
 		hostdata->cmd = NULL;
-		
+
 		if(reselection_id == 0) {
 			if(hostdata->reselection_id == 0xff) {
 				printk(KERN_ERR "scsi%d: Invalid reselection during selection!!\n", host->host_no);
@@ -1233,7 +1233,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 				reselection_id = hostdata->reselection_id;
 			}
 		} else {
-			
+
 			/* convert to real ID */
 			reselection_id = bitmap_to_number(reselection_id);
 		}
@@ -1251,7 +1251,7 @@ process_script_interrupt(__u32 dsps, __u32 dsp, struct scsi_cmnd *SCp,
 		 * a return here will re-run the queued command slot
 		 * that may have been interrupted by the initial selection */
 		DEBUG((" SELECTION COMPLETED\n"));
-	} else if((dsps & 0xfffff0f0) == A_MSG_IN) { 
+	} else if((dsps & 0xfffff0f0) == A_MSG_IN) {
 		resume_offset = process_message(host, hostdata, SCp,
 						dsp, dsps);
 	} else if((dsps &  0xfffff000) == 0) {
@@ -1304,7 +1304,7 @@ process_selection(struct Scsi_Host *host, __u32 dsp)
 
 		/* Take out our own ID */
 		id &= ~(1<<host->this_id);
-		if(id != 0) 
+		if(id != 0)
 			break;
 		udelay(5);
 	}
@@ -1322,7 +1322,7 @@ process_selection(struct Scsi_Host *host, __u32 dsp)
 		struct NCR_700_command_slot *slot =
 			(struct NCR_700_command_slot *)SCp->host_scribble;
 		DEBUG(("  ID %d WARNING: RESELECTION OF BUSY HOST, saving cmd %p, slot %p, addr %x [%04x], resume %x!\n", id, hostdata->cmd, slot, dsp, dsp - hostdata->pScript, resume_offset));
-		
+
 		switch(dsp - hostdata->pScript) {
 		case Ent_Disconnect1:
 		case Ent_Disconnect2:
@@ -1344,7 +1344,7 @@ process_selection(struct Scsi_Host *host, __u32 dsp)
 		case Ent_Finish2:
 			process_script_interrupt(A_GOOD_STATUS_AFTER_STATUS, dsp, SCp, host, hostdata);
 			break;
-			
+
 		default:
 			slot->state = NCR_700_SLOT_QUEUED;
 			break;
@@ -1547,7 +1547,7 @@ NCR_700_intr(int irq, void *dev_id)
 			/* clear all the negotiated parameters */
 			__shost_for_each_device(SDp, host)
 				NCR_700_clear_flag(SDp, ~0);
-			
+
 			/* clear all the slots and their pending commands */
 			for(i = 0; i < NCR_700_COMMAND_SLOTS_PER_HOST; i++) {
 				struct scsi_cmnd *SCp;
@@ -1556,7 +1556,7 @@ NCR_700_intr(int irq, void *dev_id)
 
 				if(slot->state == NCR_700_SLOT_FREE)
 					continue;
-				
+
 				SCp = slot->cmnd;
 				printk(KERN_ERR " failing command because of reset, slot %p, cmnd %p\n",
 				       slot, SCp);
@@ -1620,7 +1620,7 @@ NCR_700_intr(int irq, void *dev_id)
 				data_transfer += residual;
 
 				if(data_transfer != 0) {
-					int count; 
+					int count;
 					__u32 pAddr;
 
 					SGcount--;
@@ -1680,7 +1680,7 @@ NCR_700_intr(int irq, void *dev_id)
 			NCR_700_scsi_done(hostdata, SCp, DID_ERROR<<16);
 		}
 
-		
+
 		/* NOTE: selection interrupt processing MUST occur
 		 * after script interrupt processing to correctly cope
 		 * with the case where we process a disconnect and
@@ -1718,7 +1718,7 @@ NCR_700_intr(int irq, void *dev_id)
 		DEBUG(("Attempting to resume at %x\n", resume_offset));
 		NCR_700_clear_fifo(host);
 		NCR_700_writel(resume_offset, host, DSP_REG);
-	} 
+	}
 	/* There is probably a technical no-no about this: If we're a
 	 * shared interrupt and we got this interrupt because the
 	 * other device needs servicing not us, we're still going to
@@ -1732,7 +1732,7 @@ NCR_700_intr(int irq, void *dev_id)
 			 * position we left off */
 			int j = (i + hostdata->saved_slot_position)
 				% NCR_700_COMMAND_SLOTS_PER_HOST;
-			
+
 			if(hostdata->slots[j].state != NCR_700_SLOT_QUEUED)
 				continue;
 			if(NCR_700_start_command(hostdata->slots[j].cmnd)) {
@@ -1752,7 +1752,7 @@ NCR_700_intr(int irq, void *dev_id)
 
 static int NCR_700_queuecommand_lck(struct scsi_cmnd *SCp)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SCp->device->host->hostdata[0];
 	__u32 move_ins;
 	struct NCR_700_command_slot *slot;
@@ -1852,7 +1852,7 @@ static int NCR_700_queuecommand_lck(struct scsi_cmnd *SCp)
 		default:
 			printk(KERN_ERR "53c700: Unknown command for data direction ");
 			scsi_print_command(SCp);
-			
+
 			move_ins = 0;
 			break;
 		case DMA_NONE:
@@ -1937,7 +1937,7 @@ STATIC int
 NCR_700_host_reset(struct scsi_cmnd * SCp)
 {
 	DECLARE_COMPLETION_ONSTACK(complete);
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SCp->device->host->hostdata[0];
 
 	scmd_printk(KERN_INFO, SCp,
@@ -1975,9 +1975,9 @@ STATIC void
 NCR_700_set_period(struct scsi_target *STp, int period)
 {
 	struct Scsi_Host *SHp = dev_to_shost(STp->dev.parent);
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SHp->hostdata[0];
-	
+
 	if(!hostdata->fast)
 		return;
 
@@ -1994,11 +1994,11 @@ STATIC void
 NCR_700_set_offset(struct scsi_target *STp, int offset)
 {
 	struct Scsi_Host *SHp = dev_to_shost(STp->dev.parent);
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SHp->hostdata[0];
 	int max_offset = hostdata->chip710
 		? NCR_710_MAX_OFFSET : NCR_700_MAX_OFFSET;
-	
+
 	if(!hostdata->fast)
 		return;
 
@@ -2031,7 +2031,7 @@ NCR_700_slave_alloc(struct scsi_device *SDp)
 STATIC int
 NCR_700_slave_configure(struct scsi_device *SDp)
 {
-	struct NCR_700_Host_Parameters *hostdata = 
+	struct NCR_700_Host_Parameters *hostdata =
 		(struct NCR_700_Host_Parameters *)SDp->host->hostdata[0];
 
 	/* to do here: allocate memory; build a queue_full list */
@@ -2071,7 +2071,7 @@ NCR_700_show_active_tags(struct device *dev, struct device_attribute *attr, char
 {
 	struct scsi_device *SDp = to_scsi_device(dev);
 
-	return snprintf(buf, 20, "%d\n", NCR_700_get_depth(SDp));
+	return sysfs_emit(buf, "%d\n", NCR_700_get_depth(SDp));
 }
 
 static struct device_attribute NCR_700_active_tags_attr = {
-- 
2.43.0.275.g3460e3d667-goog

[PATCH 5/5] scsi: aacraid: aachba: Replace snprintf() with the safer scnprintf() variant

[Lee Jones]

There is a general misunderstanding amongst engineers that {v}snprintf()
returns the length of the data *actually* encoded into the destination
array.  However, as per the C99 standard {v}snprintf() really returns
the length of the data that *would have been* written if there were
enough space for it.  This misunderstanding has led to buffer-overruns
in the past.  It's generally considered safer to use the {v}scnprintf()
variants in their place (or even sprintf() in simple cases).  So let's
do that.

Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Adaptec OEM Raid Solutions <aacraid@microsemi.com>
Cc: "PMC-Sierra, Inc" <aacraid@pmc-sierra.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/scsi/aacraid/aachba.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 70e1cac1975eb..b22857c6f3f4f 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -1099,7 +1099,7 @@ static void get_container_serial_callback(void *context, struct fib * fibptr)
 			sp[0] = INQD_PDT_DA;
 			sp[1] = scsicmd->cmnd[2];
 			sp[2] = 0;
-			sp[3] = snprintf(sp+4, sizeof(sp)-4, "%08X",
+			sp[3] = scnprintf(sp+4, sizeof(sp)-4, "%08X",
 				le32_to_cpu(get_serial_reply->uid));
 			scsi_sg_copy_from_buffer(scsicmd, sp,
 						 sizeof(sp));
@@ -1169,8 +1169,8 @@ static int setinqserial(struct aac_dev *dev, void *data, int cid)
 	/*
 	 *	This breaks array migration.
 	 */
-	return snprintf((char *)(data), sizeof(struct scsi_inq) - 4, "%08X%02X",
-			le32_to_cpu(dev->adapter_info.serial[0]), cid);
+	return scnprintf((char *)(data), sizeof(struct scsi_inq) - 4, "%08X%02X",
+			 le32_to_cpu(dev->adapter_info.serial[0]), cid);
 }
 
 static inline void set_sense(struct sense_data *sense_data, u8 sense_key,
-- 
2.43.0.275.g3460e3d667-goog

Re: [PATCH 0/5] scsi: Replace {v}snprintf() variants with safer alternatives

[Martin K. Petersen]

Lee,

> For a far better description of the problem than I could author, see
> Jon's write-up on LWN [1] and/or Alex's on the Kernel Self Protection
> Project [1].

Applied to 6.9/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/5] scsi: Replace {v}snprintf() variants with safer alternatives

[Martin K. Petersen]

On Thu, 11 Jan 2024 13:17:21 +0000, Lee Jones wrote:

> For a far better description of the problem than I could author, see
> Jon's write-up on LWN [1] and/or Alex's on the Kernel Self Protection
> Project [1].
> 
> [0] https://lwn.net/Articles/69419/
> [1] https://github.com/KSPP/linux/issues/105
> 
> [...]

Applied to 6.9/scsi-queue, thanks!

[1/5] scsi: 3w-9xxx: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()
      https://git.kernel.org/mkp/scsi/c/a977c8158a42
[2/5] scsi: 3w-sas: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()
      https://git.kernel.org/mkp/scsi/c/7eaa48e9e497
[3/5] scsi: 3w-xxxx: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()
      https://git.kernel.org/mkp/scsi/c/30cc6aa09eee
[4/5] scsi: 53c700: Remove snprintf() from sysfs call-backs and replace with sysfs_emit()
      https://git.kernel.org/mkp/scsi/c/f615c74de383
[5/5] scsi: aacraid: aachba: Replace snprintf() with the safer scnprintf() variant
      https://git.kernel.org/mkp/scsi/c/bc978cc18d46

-- 
Martin K. Petersen	Oracle Linux Engineering