Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
On Fri, 25 May 2001, Rasmus Andersen wrote:
> On Fri, May 25, 2001 at 01:47:52PM -0700, Andre Hedrick wrote:
> >
> > Not valid because the jump to that part of the code is protected.
> > If a polling response for a valid status and no timeout, is detected then
> > it attempts to the command for real only after success or a test.
> >
> > Otherwise it would be valid.
/*
* try_to_identify() sends an ATA(PI) IDENTIFY request to a drive
* and waits for a response. It also monitors irqs while this is
* happening, in hope of automatically determining which one is
* being used by the interface.
*
* Returns: 0 device was identified
* 1 device timed-out (no response to identify request)
* 2 device aborted the command (refused to identify itself)
*/
static int actual_try_to_identify (ide_drive_t *drive, byte cmd)
{
if (OK_STAT(GET_STAT(),DRQ_STAT,BAD_R_STAT)) {
unsigned long flags;
__save_flags(flags);/* local CPU only */
__cli();/* local CPU only; some systems need this */
do_identify(drive, cmd); /* drive returned ID */
rc = 0; /* drive responded with ID */
(void) GET_STAT(); /* clear drive IRQ */
__restore_flags(flags); /* local CPU only */
} else
rc = 2; /* drive refused ID */
return rc;
}
If you get_stat returns a bad status we do not go into the test loop.
> So the EXABYTE case should return normally, I take (otherwise you
> confused me good;))? The patch below drops this change and keeps
> the rest.
/*
* probe_for_drive() tests for existence of a given drive using do_probe().
*
* Returns: 0 no device was found
* 1 device was found (note: drive->present might still be 0)
*/
static inline byte probe_for_drive (ide_drive_t *drive)
This calls out for the poke in the ribbon, and if it fail anywhere we
never get to the top function for the kmallocs.
Does that help?
This is potentially valid for modules but if it built in and you are out
of memory before partition checkinggo buy some memory. Also if you
are adding devices with a system under hard load eating/using memory to
that scale, you will have other issues to go KABOOM.
Will look at it for verification and run a gedanken process and get back
about it.
>
> --- linux-244-ac16-clean/drivers/ide/ide-probe.c Fri May 25 21:11:08 2001
> +++ linux-244-ac16/drivers/ide/ide-probe.cFri May 25 22:54:15 2001
> @@ -58,6 +58,11 @@
> struct hd_driveid *id;
>
> id = drive->id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
>interrupts disabled! */
> +if (!id) {
> +printk(KERN_WARNING "(ide-probe::do_identify) Out of memory.\n");
> +goto err_kmalloc;
> +}
> +
> ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
>id info */
> ide__sti(); /* local CPU only */
> ide_fix_driveid(id);
> @@ -76,8 +81,7 @@
> if ((id->model[0] == 'P' && id->model[1] == 'M')
>|| (id->model[0] == 'S' && id->model[1] == 'K')) {
> printk("%s: EATA SCSI HBA %.10s\n", drive->name, id->model);
> - drive->present = 0;
> - return;
> +goto err_misc;
> }
> #endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
>
> @@ -111,8 +115,7 @@
> #ifdef CONFIG_BLK_DEV_PDC4030
> if (HWIF(drive)->channel == 1 && HWIF(drive)->chipset == ide_pdc4030) {
> printk(" -- not supported on 2nd Promise port\n");
> - drive->present = 0;
> - return;
> +goto err_misc;
> }
> #endif /* CONFIG_BLK_DEV_PDC4030 */
> switch (type) {
> @@ -174,6 +177,12 @@
> printk("ATA DISK drive\n");
> QUIRK_LIST(HWIF(drive),drive);
> return;
> +
> +err_misc:
> +kfree(id);
> +err_kmalloc:
> +drive->present = 0;
> +return;
> }
>
> /*
> @@ -759,11 +768,23 @@
> }
> minors= units * (1< gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
> - gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
> - gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
> - bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
> - max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
> - max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
> +if (!gd)
> +goto err_kmalloc_gd;
> +gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
> +if (!gd->sizes)
> +goto err_kmalloc_gd_sizes;
> +gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
> +if (!gd->part)
> +goto err_kmalloc_gd_part;
> +bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
>
Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
On Fri, May 25, 2001 at 01:47:52PM -0700, Andre Hedrick wrote:
>
> Not valid because the jump to that part of the code is protected.
> If a polling response for a valid status and no timeout, is detected then
> it attempts to the command for real only after success or a test.
>
> Otherwise it would be valid.
So the EXABYTE case should return normally, I take (otherwise you
confused me good;))? The patch below drops this change and keeps
the rest.
--- linux-244-ac16-clean/drivers/ide/ide-probe.cFri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.c Fri May 25 22:54:15 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive->id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING "(ide-probe::do_identify) Out of memory.\n");
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id->model[0] == 'P' && id->model[1] == 'M')
|| (id->model[0] == 'S' && id->model[1] == 'K')) {
printk("%s: EATA SCSI HBA %.10s\n", drive->name, id->model);
- drive->present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)->channel == 1 && HWIF(drive)->chipset == ide_pdc4030) {
printk(" -- not supported on 2nd Promise port\n");
- drive->present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk("ATA DISK drive\n");
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive->present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd->sizes)
+goto err_kmalloc_gd_sizes;
+gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd->part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_sect)
+goto err_kmalloc_max_sect;
+max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_ra)
+goto err_kmalloc_max_ra;
memset(gd->part, 0, minors * sizeof(struct hd_struct));
@@ -816,6 +837,21 @@
devfs_mk_dir (ide_devfs_handle, name, NULL);
}
}
+return;
+
+err_kmalloc_max_ra:
+kfree(max_sect);
+err_kmalloc_max_sect:
+kfree(bs);
+err_kmalloc_gs:
+kfree(gd->part);
+err_kmalloc_gd_part:
+kfree(gd->sizes);
+err_kmalloc_gd_sizes:
+kfree(gd);
+err_kmalloc_gd:
+printk(KERN_WARNING "(ide::init_gendisk) Out of memory\n");
+return;
}
static int hwif_init (ide_hwif_t *hwif)
Regards,
Rasmus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
Not valid because the jump to that part of the code is protected.
If a polling response for a valid status and no timeout, is detected then
it attempts to the command for real only after success or a test.
Otherwise it would be valid.
Cheers,
On Fri, 25 May 2001, Rasmus Andersen wrote:
> Date: Fri, 25 May 2001 22:18:13 +0200
> From: Rasmus Andersen <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]
> Subject: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
>
> Hi.
>
> The following patch adds a number of checks for kmalloc returns
> to drivers/ide/ide-probe.c. It applies against ac16.
>
> One comment: This patch adds 'drive-present = 0' to the
> code path for the EXABYTE case. I could not discern if this was a
> shortcoming of the original code or not. Please comment.
>
>
> --- linux-244-ac16-clean/drivers/ide/ide-probe.c Fri May 25 21:11:08 2001
> +++ linux-244-ac16/drivers/ide/ide-probe.cFri May 25 21:51:08 2001
> @@ -58,6 +58,11 @@
> struct hd_driveid *id;
>
> id = drive->id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
>interrupts disabled! */
> +if (!id) {
> +printk(KERN_WARNING "(ide-probe::do_identify) Out of memory.\n");
> +goto err_kmalloc;
> +}
> +
> ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
>id info */
> ide__sti(); /* local CPU only */
> ide_fix_driveid(id);
> @@ -76,8 +81,7 @@
> if ((id->model[0] == 'P' && id->model[1] == 'M')
>|| (id->model[0] == 'S' && id->model[1] == 'K')) {
> printk("%s: EATA SCSI HBA %.10s\n", drive->name, id->model);
> - drive->present = 0;
> - return;
> +goto err_misc;
> }
> #endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
>
> @@ -96,7 +100,7 @@
> ide_fixstring (id->serial_no, sizeof(id->serial_no), bswap);
>
> if (strstr(id->model, "E X A B Y T E N E S T"))
> - return;
> +goto err_misc;
>
> id->model[sizeof(id->model)-1] = '\0'; /* we depend on this a lot! */
> printk("%s: %s, ", drive->name, id->model);
> @@ -111,8 +115,7 @@
> #ifdef CONFIG_BLK_DEV_PDC4030
> if (HWIF(drive)->channel == 1 && HWIF(drive)->chipset == ide_pdc4030) {
> printk(" -- not supported on 2nd Promise port\n");
> - drive->present = 0;
> - return;
> +goto err_misc;
> }
> #endif /* CONFIG_BLK_DEV_PDC4030 */
> switch (type) {
> @@ -174,6 +177,12 @@
> printk("ATA DISK drive\n");
> QUIRK_LIST(HWIF(drive),drive);
> return;
> +
> +err_misc:
> +kfree(id);
> +err_kmalloc:
> +drive->present = 0;
> +return;
> }
>
> /*
> @@ -759,11 +768,23 @@
> }
> minors= units * (1< gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
> - gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
> - gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
> - bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
> - max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
> - max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
> +if (!gd)
> +goto err_kmalloc_gd;
> +gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
> +if (!gd->sizes)
> +goto err_kmalloc_gd_sizes;
> +gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
> +if (!gd->part)
> +goto err_kmalloc_gd_part;
> +bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
> +if (!bs)
> +goto err_kmalloc_gs;
> +max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
> +if (!max_sect)
> +goto err_kmalloc_max_sect;
> +max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
> +if (!max_ra)
> +goto err_kmalloc_max_ra;
>
> memset(gd->part, 0, minors * sizeof(struct hd_struct));
>
> @@ -816,6 +837,21 @@
> devfs_mk_dir (ide_devfs_handle, name, NULL);
> }
> }
> +return;
> +
> +err_kmalloc_max_ra:
> +kfree(max_sect);
> +err_kmalloc_max_sect:
> +kfree(bs);
> +err_kmalloc_gs:
> +kfree(gd->part);
> +err_kmalloc_gd_part:
> +kfree(gd-&g
[PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
Hi.
The following patch adds a number of checks for kmalloc returns
to drivers/ide/ide-probe.c. It applies against ac16.
One comment: This patch adds 'drive-present = 0' to the
code path for the EXABYTE case. I could not discern if this was a
shortcoming of the original code or not. Please comment.
--- linux-244-ac16-clean/drivers/ide/ide-probe.cFri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.c Fri May 25 21:51:08 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive->id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING "(ide-probe::do_identify) Out of memory.\n");
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id->model[0] == 'P' && id->model[1] == 'M')
|| (id->model[0] == 'S' && id->model[1] == 'K')) {
printk("%s: EATA SCSI HBA %.10s\n", drive->name, id->model);
- drive->present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -96,7 +100,7 @@
ide_fixstring (id->serial_no, sizeof(id->serial_no), bswap);
if (strstr(id->model, "E X A B Y T E N E S T"))
- return;
+goto err_misc;
id->model[sizeof(id->model)-1] = '\0'; /* we depend on this a lot! */
printk("%s: %s, ", drive->name, id->model);
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)->channel == 1 && HWIF(drive)->chipset == ide_pdc4030) {
printk(" -- not supported on 2nd Promise port\n");
- drive->present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk("ATA DISK drive\n");
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive->present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd->sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd->sizes)
+goto err_kmalloc_gd_sizes;
+gd->part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd->part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_sect)
+goto err_kmalloc_max_sect;
+max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_ra)
+goto err_kmalloc_max_ra;
memset(gd->part, 0, minors * sizeof(struct hd_struct));
@@ -816,6 +837,21 @@
devfs_mk_dir (ide_devfs_handle, name, NULL);
}
}
+return;
+
+err_kmalloc_max_ra:
+kfree(max_sect);
+err_kmalloc_max_sect:
+kfree(bs);
+err_kmalloc_gs:
+kfree(gd->part);
+err_kmalloc_gd_part:
+kfree(gd->sizes);
+err_kmalloc_gd_sizes:
+kfree(gd);
+err_kmalloc_gd:
+printk(KERN_WARNING "(ide::init_gendisk) Out of memory\n");
+return;
}
static int hwif_init (ide_hwif_t *hwif)
--
regards,
Rasmus([EMAIL PROTECTED])
An Emacs reference mug is what I want. It would hold ten gallons of
coffee. -- Steve VanDevender
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
Hi.
The following patch adds a number of checks for kmalloc returns
to drivers/ide/ide-probe.c. It applies against ac16.
One comment: This patch adds 'drive-present = 0' to the
code path for the EXABYTE case. I could not discern if this was a
shortcoming of the original code or not. Please comment.
--- linux-244-ac16-clean/drivers/ide/ide-probe.cFri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.c Fri May 25 21:51:08 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive-id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING (ide-probe::do_identify) Out of memory.\n);
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id-model[0] == 'P' id-model[1] == 'M')
|| (id-model[0] == 'S' id-model[1] == 'K')) {
printk(%s: EATA SCSI HBA %.10s\n, drive-name, id-model);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -96,7 +100,7 @@
ide_fixstring (id-serial_no, sizeof(id-serial_no), bswap);
if (strstr(id-model, E X A B Y T E N E S T))
- return;
+goto err_misc;
id-model[sizeof(id-model)-1] = '\0'; /* we depend on this a lot! */
printk(%s: %s, , drive-name, id-model);
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)-channel == 1 HWIF(drive)-chipset == ide_pdc4030) {
printk( -- not supported on 2nd Promise port\n);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk(ATA DISK drive\n);
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive-present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1PARTN_BITS);
gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
- gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
- gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd-sizes)
+goto err_kmalloc_gd_sizes;
+gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd-part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_sect)
+goto err_kmalloc_max_sect;
+max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_ra)
+goto err_kmalloc_max_ra;
memset(gd-part, 0, minors * sizeof(struct hd_struct));
@@ -816,6 +837,21 @@
devfs_mk_dir (ide_devfs_handle, name, NULL);
}
}
+return;
+
+err_kmalloc_max_ra:
+kfree(max_sect);
+err_kmalloc_max_sect:
+kfree(bs);
+err_kmalloc_gs:
+kfree(gd-part);
+err_kmalloc_gd_part:
+kfree(gd-sizes);
+err_kmalloc_gd_sizes:
+kfree(gd);
+err_kmalloc_gd:
+printk(KERN_WARNING (ide::init_gendisk) Out of memory\n);
+return;
}
static int hwif_init (ide_hwif_t *hwif)
--
regards,
Rasmus([EMAIL PROTECTED])
An Emacs reference mug is what I want. It would hold ten gallons of
coffee. -- Steve VanDevender
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
Not valid because the jump to that part of the code is protected.
If a polling response for a valid status and no timeout, is detected then
it attempts to the command for real only after success or a test.
Otherwise it would be valid.
Cheers,
On Fri, 25 May 2001, Rasmus Andersen wrote:
Date: Fri, 25 May 2001 22:18:13 +0200
From: Rasmus Andersen [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Subject: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
Hi.
The following patch adds a number of checks for kmalloc returns
to drivers/ide/ide-probe.c. It applies against ac16.
One comment: This patch adds 'drive-present = 0' to the
code path for the EXABYTE case. I could not discern if this was a
shortcoming of the original code or not. Please comment.
--- linux-244-ac16-clean/drivers/ide/ide-probe.c Fri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.cFri May 25 21:51:08 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive-id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING (ide-probe::do_identify) Out of memory.\n);
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id-model[0] == 'P' id-model[1] == 'M')
|| (id-model[0] == 'S' id-model[1] == 'K')) {
printk(%s: EATA SCSI HBA %.10s\n, drive-name, id-model);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -96,7 +100,7 @@
ide_fixstring (id-serial_no, sizeof(id-serial_no), bswap);
if (strstr(id-model, E X A B Y T E N E S T))
- return;
+goto err_misc;
id-model[sizeof(id-model)-1] = '\0'; /* we depend on this a lot! */
printk(%s: %s, , drive-name, id-model);
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)-channel == 1 HWIF(drive)-chipset == ide_pdc4030) {
printk( -- not supported on 2nd Promise port\n);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk(ATA DISK drive\n);
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive-present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1PARTN_BITS);
gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
- gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
- gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd-sizes)
+goto err_kmalloc_gd_sizes;
+gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd-part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_sect)
+goto err_kmalloc_max_sect;
+max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_ra)
+goto err_kmalloc_max_ra;
memset(gd-part, 0, minors * sizeof(struct hd_struct));
@@ -816,6 +837,21 @@
devfs_mk_dir (ide_devfs_handle, name, NULL);
}
}
+return;
+
+err_kmalloc_max_ra:
+kfree(max_sect);
+err_kmalloc_max_sect:
+kfree(bs);
+err_kmalloc_gs:
+kfree(gd-part);
+err_kmalloc_gd_part:
+kfree(gd-sizes);
+err_kmalloc_gd_sizes:
+kfree(gd);
+err_kmalloc_gd:
+printk(KERN_WARNING (ide::init_gendisk) Out of memory\n);
+return;
}
static int hwif_init (ide_hwif_t *hwif)
--
regards,
Rasmus([EMAIL PROTECTED])
An Emacs reference mug is what I want. It would hold ten gallons of
coffee. -- Steve VanDevender
Andre Hedrick
Linux ATA Development
ASL Kernel Development
-
ASL, Inc. Toll free: 1-877-ASL-3535
1757 Houret Court Fax: 1-408-941-2071
Milpitas, CA 95035
Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
On Fri, May 25, 2001 at 01:47:52PM -0700, Andre Hedrick wrote:
Not valid because the jump to that part of the code is protected.
If a polling response for a valid status and no timeout, is detected then
it attempts to the command for real only after success or a test.
Otherwise it would be valid.
So the EXABYTE case should return normally, I take (otherwise you
confused me good;))? The patch below drops this change and keeps
the rest.
--- linux-244-ac16-clean/drivers/ide/ide-probe.cFri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.c Fri May 25 22:54:15 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive-id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING (ide-probe::do_identify) Out of memory.\n);
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id-model[0] == 'P' id-model[1] == 'M')
|| (id-model[0] == 'S' id-model[1] == 'K')) {
printk(%s: EATA SCSI HBA %.10s\n, drive-name, id-model);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)-channel == 1 HWIF(drive)-chipset == ide_pdc4030) {
printk( -- not supported on 2nd Promise port\n);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk(ATA DISK drive\n);
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive-present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1PARTN_BITS);
gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
- gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
- gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd-sizes)
+goto err_kmalloc_gd_sizes;
+gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd-part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_sect)
+goto err_kmalloc_max_sect;
+max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!max_ra)
+goto err_kmalloc_max_ra;
memset(gd-part, 0, minors * sizeof(struct hd_struct));
@@ -816,6 +837,21 @@
devfs_mk_dir (ide_devfs_handle, name, NULL);
}
}
+return;
+
+err_kmalloc_max_ra:
+kfree(max_sect);
+err_kmalloc_max_sect:
+kfree(bs);
+err_kmalloc_gs:
+kfree(gd-part);
+err_kmalloc_gd_part:
+kfree(gd-sizes);
+err_kmalloc_gd_sizes:
+kfree(gd);
+err_kmalloc_gd:
+printk(KERN_WARNING (ide::init_gendisk) Out of memory\n);
+return;
}
static int hwif_init (ide_hwif_t *hwif)
Regards,
Rasmus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kmalloc checks for drivers/ide/ide-probe.c (244ac16)
On Fri, 25 May 2001, Rasmus Andersen wrote:
On Fri, May 25, 2001 at 01:47:52PM -0700, Andre Hedrick wrote:
Not valid because the jump to that part of the code is protected.
If a polling response for a valid status and no timeout, is detected then
it attempts to the command for real only after success or a test.
Otherwise it would be valid.
/*
* try_to_identify() sends an ATA(PI) IDENTIFY request to a drive
* and waits for a response. It also monitors irqs while this is
* happening, in hope of automatically determining which one is
* being used by the interface.
*
* Returns: 0 device was identified
* 1 device timed-out (no response to identify request)
* 2 device aborted the command (refused to identify itself)
*/
static int actual_try_to_identify (ide_drive_t *drive, byte cmd)
{
snip
if (OK_STAT(GET_STAT(),DRQ_STAT,BAD_R_STAT)) {
unsigned long flags;
__save_flags(flags);/* local CPU only */
__cli();/* local CPU only; some systems need this */
do_identify(drive, cmd); /* drive returned ID */
rc = 0; /* drive responded with ID */
(void) GET_STAT(); /* clear drive IRQ */
__restore_flags(flags); /* local CPU only */
} else
rc = 2; /* drive refused ID */
return rc;
}
If you get_stat returns a bad status we do not go into the test loop.
So the EXABYTE case should return normally, I take (otherwise you
confused me good;))? The patch below drops this change and keeps
the rest.
/*
* probe_for_drive() tests for existence of a given drive using do_probe().
*
* Returns: 0 no device was found
* 1 device was found (note: drive-present might still be 0)
*/
static inline byte probe_for_drive (ide_drive_t *drive)
This calls out for the poke in the ribbon, and if it fail anywhere we
never get to the top function for the kmallocs.
Does that help?
This is potentially valid for modules but if it built in and you are out
of memory before partition checkinggo buy some memory. Also if you
are adding devices with a system under hard load eating/using memory to
that scale, you will have other issues to go KABOOM.
Will look at it for verification and run a gedanken process and get back
about it.
--- linux-244-ac16-clean/drivers/ide/ide-probe.c Fri May 25 21:11:08 2001
+++ linux-244-ac16/drivers/ide/ide-probe.cFri May 25 22:54:15 2001
@@ -58,6 +58,11 @@
struct hd_driveid *id;
id = drive-id = kmalloc (SECTOR_WORDS*4, GFP_ATOMIC); /* called with
interrupts disabled! */
+if (!id) {
+printk(KERN_WARNING (ide-probe::do_identify) Out of memory.\n);
+goto err_kmalloc;
+}
+
ide_input_data(drive, id, SECTOR_WORDS);/* read 512 bytes of
id info */
ide__sti(); /* local CPU only */
ide_fix_driveid(id);
@@ -76,8 +81,7 @@
if ((id-model[0] == 'P' id-model[1] == 'M')
|| (id-model[0] == 'S' id-model[1] == 'K')) {
printk(%s: EATA SCSI HBA %.10s\n, drive-name, id-model);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_SCSI_EATA_DMA || CONFIG_SCSI_EATA_PIO */
@@ -111,8 +115,7 @@
#ifdef CONFIG_BLK_DEV_PDC4030
if (HWIF(drive)-channel == 1 HWIF(drive)-chipset == ide_pdc4030) {
printk( -- not supported on 2nd Promise port\n);
- drive-present = 0;
- return;
+goto err_misc;
}
#endif /* CONFIG_BLK_DEV_PDC4030 */
switch (type) {
@@ -174,6 +177,12 @@
printk(ATA DISK drive\n);
QUIRK_LIST(HWIF(drive),drive);
return;
+
+err_misc:
+kfree(id);
+err_kmalloc:
+drive-present = 0;
+return;
}
/*
@@ -759,11 +768,23 @@
}
minors= units * (1PARTN_BITS);
gd= kmalloc (sizeof(struct gendisk), GFP_KERNEL);
- gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
- gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
- bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_sect = kmalloc (minors*sizeof(int), GFP_KERNEL);
- max_ra= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!gd)
+goto err_kmalloc_gd;
+gd-sizes = kmalloc (minors * sizeof(int), GFP_KERNEL);
+if (!gd-sizes)
+goto err_kmalloc_gd_sizes;
+gd-part = kmalloc (minors * sizeof(struct hd_struct), GFP_KERNEL);
+if (!gd-part)
+goto err_kmalloc_gd_part;
+bs= kmalloc (minors*sizeof(int), GFP_KERNEL);
+if (!bs)
+goto err_kmalloc_gs;
+max_sect = kmalloc
