diff mbox

[V2] st: fix potential null pointer dereference.

Message ID 1447857164-2043-1-git-send-email-mlombard@redhat.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Maurizio Lombardi Nov. 18, 2015, 2:32 p.m. UTC
If cdev_add() returns an error, the code calls
cdev_del() passing the STm->cdevs[rew] pointer as parameter;
the problem is that the pointer has not been initialized yet.

This patch fixes the problem by moving the STm->cdevs[rew] pointer
initialization before the call to cdev_add().
It also sets STm->devs[rew] and STm->cdevs[rew] to NULL in
case of failure.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/scsi/st.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Tomas Henzl Nov. 19, 2015, 3:10 p.m. UTC | #1
On 18.11.2015 15:32, Maurizio Lombardi wrote:
> If cdev_add() returns an error, the code calls
> cdev_del() passing the STm->cdevs[rew] pointer as parameter;
> the problem is that the pointer has not been initialized yet.
>
> This patch fixes the problem by moving the STm->cdevs[rew] pointer
> initialization before the call to cdev_add().
> It also sets STm->devs[rew] and STm->cdevs[rew] to NULL in
> case of failure.
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
>  drivers/scsi/st.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index e0a1e52..2e52295 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -4083,6 +4083,7 @@ static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
>  	}
>  	cdev->owner = THIS_MODULE;
>  	cdev->ops = &st_fops;
> +	STm->cdevs[rew] = cdev;
>  
>  	error = cdev_add(cdev, cdev_devno, 1);
>  	if (error) {
> @@ -4091,7 +4092,6 @@ static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
>  		pr_err("st%d: Device not attached.\n", dev_num);
>  		goto out_free;
>  	}
> -	STm->cdevs[rew] = cdev;
>  
>  	i = mode << (4 - ST_NBR_MODE_BITS);
>  	snprintf(name, 10, "%s%s%s", rew ? "n" : "",
> @@ -4110,8 +4110,9 @@ static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
>  	return 0;
>  out_free:
>  	cdev_del(STm->cdevs[rew]);
> -	STm->cdevs[rew] = NULL;
>  out:
> +	STm->cdevs[rew] = NULL;
> +	STm->devs[rew] = NULL;

Reviewed-by: Tomas Henzl <thenzl@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Thumshirn Nov. 19, 2015, 3:13 p.m. UTC | #2
On Wed, 2015-11-18 at 15:32 +0100, Maurizio Lombardi wrote:
> If cdev_add() returns an error, the code calls
> cdev_del() passing the STm->cdevs[rew] pointer as parameter;
> the problem is that the pointer has not been initialized yet.
> 
> This patch fixes the problem by moving the STm->cdevs[rew] pointer
> initialization before the call to cdev_add().
> It also sets STm->devs[rew] and STm->cdevs[rew] to NULL in
> case of failure.
> 
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
>  drivers/scsi/st.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index e0a1e52..2e52295 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -4083,6 +4083,7 @@ static int create_one_cdev(struct scsi_tape
> *tape, int mode, int rew)
>  	}
>  	cdev->owner = THIS_MODULE;
>  	cdev->ops = &st_fops;
> +	STm->cdevs[rew] = cdev;
>  
>  	error = cdev_add(cdev, cdev_devno, 1);
>  	if (error) {
> @@ -4091,7 +4092,6 @@ static int create_one_cdev(struct scsi_tape
> *tape, int mode, int rew)
>  		pr_err("st%d: Device not attached.\n", dev_num);
>  		goto out_free;
>  	}
> -	STm->cdevs[rew] = cdev;
>  
>  	i = mode << (4 - ST_NBR_MODE_BITS);
>  	snprintf(name, 10, "%s%s%s", rew ? "n" : "",
> @@ -4110,8 +4110,9 @@ static int create_one_cdev(struct scsi_tape
> *tape, int mode, int rew)
>  	return 0;
>  out_free:
>  	cdev_del(STm->cdevs[rew]);
> -	STm->cdevs[rew] = NULL;
>  out:
> +	STm->cdevs[rew] = NULL;
> +	STm->devs[rew] = NULL;
>  	return error;
>  }
>  

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kai Mäkisara (Kolumbus) Nov. 24, 2015, 7:59 p.m. UTC | #3
> On 18.11.2015, at 16.32, Maurizio Lombardi <mlombard@redhat.com> wrote:
> 
> If cdev_add() returns an error, the code calls
> cdev_del() passing the STm->cdevs[rew] pointer as parameter;
> the problem is that the pointer has not been initialized yet.
> 
> This patch fixes the problem by moving the STm->cdevs[rew] pointer
> initialization before the call to cdev_add().
> It also sets STm->devs[rew] and STm->cdevs[rew] to NULL in
> case of failure.
> 
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>

Acked-by: Kai Mäkisara <kai.makisara@kolumbus.fi>

Thanks,
Kai

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e0a1e52..2e52295 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -4083,6 +4083,7 @@  static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
 	}
 	cdev->owner = THIS_MODULE;
 	cdev->ops = &st_fops;
+	STm->cdevs[rew] = cdev;
 
 	error = cdev_add(cdev, cdev_devno, 1);
 	if (error) {
@@ -4091,7 +4092,6 @@  static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
 		pr_err("st%d: Device not attached.\n", dev_num);
 		goto out_free;
 	}
-	STm->cdevs[rew] = cdev;
 
 	i = mode << (4 - ST_NBR_MODE_BITS);
 	snprintf(name, 10, "%s%s%s", rew ? "n" : "",
@@ -4110,8 +4110,9 @@  static int create_one_cdev(struct scsi_tape *tape, int mode, int rew)
 	return 0;
 out_free:
 	cdev_del(STm->cdevs[rew]);
-	STm->cdevs[rew] = NULL;
 out:
+	STm->cdevs[rew] = NULL;
+	STm->devs[rew] = NULL;
 	return error;
 }