diff mbox series

scsi: sr: add handling of memory allocation failure in get_capabilities

Message ID 20220413100008.522912-1-lienze@kylinos.cn (mailing list archive)
State Changes Requested
Headers show
Series scsi: sr: add handling of memory allocation failure in get_capabilities | expand

Commit Message

Enze Li April 13, 2022, 10 a.m. UTC 
The function get_capabilities() has the possibility of failing to
allocate transfer buffer, but it does not currently handle this, which
can lead to exceptions when accessing the buffer.

This patch adds handling when memory allocation fails.

Signed-off-by: Enze Li <lienze@kylinos.cn>
---
 drivers/scsi/sr.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Martin K. Petersen April 26, 2022, 12:46 p.m. UTC | #1 
Enze,

> @@ -669,8 +669,9 @@ static int sr_probe(struct device *dev)
>  
>  	sdev->sector_size = 2048;	/* A guess, just in case */
>  
> -	/* FIXME: need to handle a get_capabilities failure properly ?? */
> -	get_capabilities(cd);
> +	error = -ENOMEM;
> +	if (get_capabilities(cd))
> +		goto fail_put;

Shouldn't this be 'goto fail_minor;'?
Enze Li April 27, 2022, 3:03 a.m. UTC | #2 
Hi Martin,

Thank you for the review.  I've fixed the issue you pointed out.
v2 of this patch is here:
https://lore.kernel.org/all/20220427025647.298358-1-lienze@kylinos.cn/

Thanks,
Enze

>Enze,
>
>> @@ -669,8 +669,9 @@ static int sr_probe(struct device *dev)
>>  
>>  	sdev->sector_size = 2048;	/* A guess, just in case */
>>  
>> -	/* FIXME: need to handle a get_capabilities failure properly
>> ?? */
>> -	get_capabilities(cd);
>> +	error = -ENOMEM;
>> +	if (get_capabilities(cd))
>> +		goto fail_put;
>
>Shouldn't this be 'goto fail_minor;'?
>
>-- 
>Martin K. Petersen	Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index cbd92891a762..868f88b1f4d4 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -113,7 +113,7 @@  static int sr_open(struct cdrom_device_info *, int);
 static void sr_release(struct cdrom_device_info *);
 
 static void get_sectorsize(struct scsi_cd *);
-static void get_capabilities(struct scsi_cd *);
+static int get_capabilities(struct scsi_cd *);
 
 static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 				    unsigned int clearing, int slot);
@@ -669,8 +669,9 @@  static int sr_probe(struct device *dev)
 
 	sdev->sector_size = 2048;	/* A guess, just in case */
 
-	/* FIXME: need to handle a get_capabilities failure properly ?? */
-	get_capabilities(cd);
+	error = -ENOMEM;
+	if (get_capabilities(cd))
+		goto fail_put;
 	sr_vendor_init(cd);
 
 	set_capacity(disk, cd->capacity);
@@ -794,7 +795,7 @@  static void get_sectorsize(struct scsi_cd *cd)
 	return;
 }
 
-static void get_capabilities(struct scsi_cd *cd)
+static int get_capabilities(struct scsi_cd *cd)
 {
 	unsigned char *buffer;
 	struct scsi_mode_data data;
@@ -819,7 +820,7 @@  static void get_capabilities(struct scsi_cd *cd)
 	buffer = kmalloc(512, GFP_KERNEL);
 	if (!buffer) {
 		sr_printk(KERN_ERR, cd, "out of memory.\n");
-		return;
+		return -ENOMEM;
 	}
 
 	/* eat unit attentions */
@@ -839,7 +840,7 @@  static void get_capabilities(struct scsi_cd *cd)
 				 CDC_MRW | CDC_MRW_W | CDC_RAM);
 		kfree(buffer);
 		sr_printk(KERN_INFO, cd, "scsi-1 drive");
-		return;
+		return 0;
 	}
 
 	n = data.header_length + data.block_descriptor_length;
@@ -898,6 +899,7 @@  static void get_capabilities(struct scsi_cd *cd)
 	}
 
 	kfree(buffer);
+	return 0;
 }
 
 /*