Hello,
In sas_smp_get_phy_events() we never test if the call to

alloc_smp_req(RPEL_REQ_SIZE) succeeds or fails. That means we run

the risk of dereferencing a NULL pointer if it does fail. Far

better to test if we got NULL back and in that case return -ENOMEM

just as we already do for the other memory allocation in that

function.

This patch reworks the memory allocation a bit to deal with it

(compile tested only).
Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>

---
 drivers/scsi/libsas/sas_expander.c |   11 +++++++++--

 1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c

index b500f0c..85f5145 100644

--- a/drivers/scsi/libsas/sas_expander.c

+++ b/drivers/scsi/libsas/sas_expander.c

@@ -507,14 +507,21 @@ static int sas_dev_present_in_domain(struct asd_sas_port *port,

 int sas_smp_get_phy_events(struct sas_phy *phy)

 {

 	int res;

+	u8 *req;

+	u8 *resp;

 	struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent);

 	struct domain_device *dev = sas_find_dev_by_rphy(rphy);

-	u8 *req = alloc_smp_req(RPEL_REQ_SIZE);

-	u8 *resp = kzalloc(RPEL_RESP_SIZE, GFP_KERNEL);
+	resp = kzalloc(RPEL_RESP_SIZE, GFP_KERNEL);

 	if (!resp)

 		return -ENOMEM;
+	req = alloc_smp_req(RPEL_REQ_SIZE);

+	if (!req) {

+        	res = -ENOMEM;

+		goto out;

+	}

+

 	req[1] = SMP_REPORT_PHY_ERR_LOG;

 	req[9] = phy->number;
  PS. Please keep me on Cc when replying.
-

Just for the sake of being the same as all the rest of the code, the

sequence should be
	req = alloc_smp_req(xxx_REQ_SIZE);

	if (!req)

		return -ENOMEM;
	resp = alloc_smp_resp(xxx_RESP_SIZE);

	if (!resp) {

		kfree(req);

		return -ENOMEM;

	}
(allocate request then response).
It looks like disc_resp could use a little love too (it's using the req

alloc routines).
James
-

Fair enough. It makes the code a bit larger though : 
 My way, as per the original patch:

    text    data     bss     dec     hex filename

   13820       0       8   13828    3604 drivers/scsi/libsas/sas_expander.o

 Your way, as per this patch:

    text    data     bss     dec     hex filename

   13832       0       8   13840    3610 drivers/scsi/libsas/sas_expander.o
I hope this patch is acceptable : 
In sas_smp_get_phy_events() we never test if the call to

alloc_smp_req(RPEL_REQ_SIZE) succeeds or fails. That means we run

the risk of dereferencing a NULL pointer if it does fail. Far

better to test if we got NULL back and in that case return -ENOMEM

just as we already do for the other memory allocation in that

function.

This patch should take care of it (compile tested only).
Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>

---
 drivers/scsi/libsas/sas_expander.c |   13 ++++++++++---

 1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c

index b500f0c..e98d2b9 100644

--- a/drivers/scsi/libsas/sas_expander.c

+++ b/drivers/scsi/libsas/sas_expander.c

@@ -507,14 +507,21 @@ static int sas_dev_present_in_domain(struct asd_sas_port *port,

 int sas_smp_get_phy_events(struct sas_phy *phy)

 {

 	int res;

+	u8 *req;

+	u8 *resp;

 	struct sas_rphy *rphy = dev_to_rphy(phy->dev.parent);

 	struct domain_device *dev = sas_find_dev_by_rphy(rphy);

-	u8 *req = alloc_smp_req(RPEL_REQ_SIZE);

-	u8 *resp = kzalloc(RPEL_RESP_SIZE, GFP_KERNEL);
-	if (!resp)

+	req = alloc_smp_req(RPEL_REQ_SIZE);

+	if (!req)

 		return -ENOMEM;
+	resp = alloc_smp_resp(RPEL_RESP_SIZE);

+	if (!resp) {

+		kfree(req);

+		return -ENOMEM;

+	}

+

 	req[1] = SMP_REPORT_PHY_ERR_LOG;

I'll take a look at that later.
--

Jesper Juhl <jesper.juhl@gmail.com>

Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html

Plain text mails only, please      http://www.expita.com/nomime.html
-

...
[ message continues ]