linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@suse.de>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>
Subject: target: Fix excessive stack usage
Date: Tue, 04 Jan 2011 18:29:21 -0600	[thread overview]
Message-ID: <1294187361.7879.21.camel@mulgrave.site> (raw)

This is showing up in compiles:

  CC [M]  drivers/target/target_core_alua.o
drivers/target/target_core_pr.c: In function '__core_scsi3_update_aptpl_buf':
drivers/target/target_core_pr.c:1952: warning: the frame size of 1124 bytes is larger than 1024 bytes

And is caused by a tmp[1024] buffer in the routine.  This looks to be
fixable with a GFP_KERNEL allocation ... although it's hard to say; the
routine is called under the dev_reservation_lock which, best as I can
tell, is only ever called with user context ... so why isn't it a mutex?
There's also no need to zero initialise an entire string buffer; as long
as the kernel prints the string (which it does in this case), it's fine
just to start it with a null.

James

---

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index b43bf8d..6c3dd82 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1857,9 +1857,9 @@ static int __core_scsi3_update_aptpl_buf(
 	struct se_portal_group *tpg;
 	struct se_subsystem_dev *su_dev = SU_DEV(dev);
 	struct t10_pr_registration *pr_reg;
-	unsigned char tmp[1024], isid_buf[32];
+	unsigned char *tmp, isid_buf[32];
 	ssize_t len = 0;
-	int reg_count = 0;
+	int reg_count = 0, ret = 0;
 
 	memset(buf, 0, pr_aptpl_buf_len);
 	/*
@@ -1870,6 +1870,10 @@ static int __core_scsi3_update_aptpl_buf(
 				"No Registrations or Reservations\n");
 		return 0;
 	}
+	tmp = kmalloc(1024, GFP_KERNEL);
+	if (!tmp)
+		return -1;
+
 	/*
 	 * Walk the registration list..
 	 */
@@ -1877,8 +1881,8 @@ static int __core_scsi3_update_aptpl_buf(
 	list_for_each_entry(pr_reg, &T10_RES(su_dev)->registration_list,
 			pr_reg_list) {
 
-		memset(tmp, 0, 1024);
-		memset(isid_buf, 0, 32);
+		tmp[0] = '\0';
+		isid_buf[0] = '\0';
 		tpg = pr_reg->pr_reg_nacl->se_tpg;
 		lun = pr_reg->pr_reg_tg_pt_lun;
 		/*
@@ -1920,7 +1924,8 @@ static int __core_scsi3_update_aptpl_buf(
 			printk(KERN_ERR "Unable to update renaming"
 				" APTPL metadata\n");
 			spin_unlock(&T10_RES(su_dev)->registration_lock);
-			return -1;
+			ret = -1;
+			goto out;
 		}
 		len += sprintf(buf+len, "%s", tmp);
 
@@ -1938,17 +1943,20 @@ static int __core_scsi3_update_aptpl_buf(
 			printk(KERN_ERR "Unable to update renaming"
 				" APTPL metadata\n");
 			spin_unlock(&T10_RES(su_dev)->registration_lock);
-			return -1;
+			ret = -1;
+			goto out;
 		}
 		len += sprintf(buf+len, "%s", tmp);
 		reg_count++;
 	}
 	spin_unlock(&T10_RES(su_dev)->registration_lock);
 
-	 if (!(reg_count))
+	if (!(reg_count))
 		len += sprintf(buf+len, "No Registrations or Reservations");
 
-	return 0;
+ out:
+	kfree(tmp);
+	return ret;
 }
 
 static int core_scsi3_update_aptpl_buf(



             reply	other threads:[~2011-01-05  0:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-05  0:29 James Bottomley [this message]
2011-01-05 22:23 ` target: Fix excessive stack usage Nicholas A. Bellinger
2011-01-07 16:32 ` Rolf Eike Beer
2011-01-07 20:55   ` Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1294187361.7879.21.camel@mulgrave.site \
    --to=james.bottomley@suse.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).