Re: how much stack space can I use

Jens Axboe (axboe@image.dk)
Sun, 24 Oct 1999 14:44:29 +0200


--tThc/1wpZn/ma/RB
Content-Type: text/plain; charset=us-ascii

On Sun, Oct 24 1999, Keith Owens wrote:
> 7c4 ncr53c8xx_detect
> 824 dvd_read_disckey
> 824 dvd_read_manufact
> 914 mmc_ioctl
>
> mmc_ioctl is worrying. That code contains a switch statement with
> allocations on individual case statements. gcc version egcs-2.91.66
> 19990314/Linux (egcs-1.1.2 release) has consolidated all the
> allocations into a single large allocation at the start of the routine.

Here's a diff against 2.3.23 that should take care of the above
stack problems in cdrom.c. Your script does not work for me
for some reason, so I haven't checked the new results (I'm
just lazy).

-- 
*  Jens Axboe <axboe@image.dk>
*  Linux CD-ROM Maintainer
*  http://www.kernel.dk

--tThc/1wpZn/ma/RB Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="cdrom_stack.diff"

--- /tmp/linux-2.3.23-clean/drivers/cdrom/cdrom.c Sun Oct 24 14:41:38 1999 +++ drivers/cdrom/cdrom.c Sun Oct 24 14:27:17 1999 @@ -1123,11 +1123,14 @@ static int dvd_read_disckey (struct cdrom_device_info *cdi, dvd_struct *s) { int ret; - u_char buf[4 + 2048]; + u_char *buf; struct cdrom_generic_command cgc; struct cdrom_device_ops *cdo = cdi->ops; - memset(buf, 0, sizeof (buf)); + if ((buf = (u_char *) kmalloc(2060, GFP_KERNEL)) == NULL) + return -ENOMEM; + + memset(buf, 0, sizeof(buf)); init_cdrom_command(&cgc, buf, sizeof(buf)); cgc.cmd[0] = GPCMD_READ_DVD_STRUCTURE; cgc.cmd[7] = s->type; @@ -1135,11 +1138,14 @@ cgc.cmd[9] = cgc.buflen & 0xff; cgc.cmd[10] = s->disckey.agid << 6; - if ((ret = cdo->generic_packet(cdi, &cgc))) + if ((ret = cdo->generic_packet(cdi, &cgc))) { + kfree(buf); return ret; + } memcpy(s->disckey.value, &buf[4], 2048); + kfree(buf); return 0; } @@ -1171,11 +1177,14 @@ static int dvd_read_manufact(struct cdrom_device_info *cdi, dvd_struct *s) { - int ret; - u_char buf[4 + 2048]; + int ret = 0; + u_char *buf; struct cdrom_generic_command cgc; struct cdrom_device_ops *cdo = cdi->ops; + if ((buf = (u_char *) kmalloc(2060, GFP_KERNEL)) == NULL) + return -ENOMEM; + memset(buf, 0, sizeof(buf)); init_cdrom_command(&cgc, buf, sizeof(buf)); cgc.cmd[0] = GPCMD_READ_DVD_STRUCTURE; @@ -1185,16 +1194,20 @@ cgc.cmd[9] = cgc.buflen & 0xff; if ((ret = cdo->generic_packet(cdi, &cgc))) - return ret; + goto out_fail; s->manufact.len = buf[0] << 8 | buf[1]; if (s->manufact.len < 0 || s->manufact.len > 2048) { - cdinfo(CD_WARNING, "Recieved invalid manufacture info length (%d)\n", s->bca.len); - return -EIO; + cdinfo(CD_WARNING, "Received invalid manufacture info length" + " (%d)\n", s->bca.len); + ret = -EIO; + goto out_fail; } memcpy(s->manufact.value, &buf[4], s->manufact.len); - return 0; +out_fail: + kfree(buf); + return ret; } static int dvd_read_struct(struct cdrom_device_info *cdi, dvd_struct *s) @@ -1952,27 +1965,49 @@ } case DVD_READ_STRUCT: { - dvd_struct s; - if (!CDROM_CAN(CDC_DVD)) - return -ENOSYS; + dvd_struct *s; + s = (dvd_struct *) kmalloc(sizeof(dvd_struct), GFP_KERNEL); + if (s == NULL) + return -ENOMEM; + if (!CDROM_CAN(CDC_DVD)) { + ret = -ENOSYS; + goto free_s; + } cdinfo(CD_DO_IOCTL, "entering DVD_READ_STRUCT\n"); - IOCTL_IN(arg, dvd_struct, s); - if ((ret = dvd_read_struct(cdi, &s))) - return ret; - IOCTL_OUT(arg, dvd_struct, s); - return 0; + if (copy_from_user(s, (dvd_struct *)arg, sizeof(dvd_struct))) { + ret = -EFAULT; + goto free_s; + } + if ((ret = dvd_read_struct(cdi, s))) + goto free_s; + if (copy_to_user((dvd_struct *)arg, s, sizeof(dvd_struct))) + ret = -EFAULT; + free_s: + kfree(s); + return ret; } case DVD_AUTH: { - dvd_authinfo ai; - if (!CDROM_CAN(CDC_DVD)) - return -ENOSYS; + dvd_authinfo *ai; + ai = (dvd_authinfo *) kmalloc(sizeof(dvd_authinfo), GFP_KERNEL); + if (ai == NULL) + return -ENOMEM; + if (!CDROM_CAN(CDC_DVD)) { + ret = -ENOSYS; + goto free_ai; + } cdinfo(CD_DO_IOCTL, "entering DVD_AUTH\n"); - IOCTL_IN(arg, dvd_authinfo, ai); - if ((ret = dvd_do_auth (cdi, &ai))) - return ret; - IOCTL_OUT(arg, dvd_authinfo, ai); - return 0; + if (copy_from_user(ai, (dvd_authinfo *) arg, sizeof(dvd_authinfo))) { + ret = -EFAULT; + goto free_ai; + } + if ((ret = dvd_do_auth (cdi, ai))) + goto free_ai; + if (copy_to_user((dvd_authinfo *) arg, ai, sizeof(dvd_authinfo))) + ret = -EFAULT; + free_ai: + kfree(ai); + return ret; } case CDROM_SEND_PACKET: {

--tThc/1wpZn/ma/RB--

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/