| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20080109103459.GC16462@skl-net.de>
Date: Wed, 9 Jan 2008 11:34:59 +0100
From: Andre Noll <maan@...temlinux.org>
To: Andi Kleen <andi@...stfloor.org>
Cc: linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
paolo.ciarrocchi@...il.com, gorcunov@...il.com
Subject: Re: [JANITOR PROPOSAL] Switch ioctl functions to ->unlocked_ioctl
On 17:40, Andi Kleen wrote:
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
Here's my take on drivers/scsi/sg.c. It's only compile-tested on x86-64.
Please review.
Andre
---
Convert sg.c to the new unlocked_ioctl entry point.
Signed-off-by: Andre Noll <maan@...temlinux.org>
---
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index f1871ea..3063307 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -48,6 +48,7 @@ static int sg_version_num = 30534; /* 2 digits for each component */
#include <linux/blkdev.h>
#include <linux/delay.h>
#include <linux/scatterlist.h>
+#include <linux/smp_lock.h>
#include "scsi.h"
#include <scsi/scsi_dbg.h>
@@ -764,20 +765,22 @@ sg_srp_done(Sg_request *srp, Sg_fd *sfp)
return done;
}
-static int
-sg_ioctl(struct inode *inode, struct file *filp,
- unsigned int cmd_in, unsigned long arg)
+static long
+sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
void __user *p = (void __user *)arg;
int __user *ip = p;
- int result, val, read_only;
+ int val, read_only;
Sg_device *sdp;
Sg_fd *sfp;
Sg_request *srp;
unsigned long iflags;
+ long result;
+ lock_kernel();
+ result = -ENXIO;
if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
- return -ENXIO;
+ goto out;
SCSI_LOG_TIMEOUT(3, printk("sg_ioctl: %s, cmd=0x%x\n",
sdp->disk->disk_name, (int) cmd_in));
read_only = (O_RDWR != (filp->f_flags & O_ACCMODE));
@@ -787,57 +790,66 @@ sg_ioctl(struct inode *inode, struct file *filp,
{
int blocking = 1; /* ignore O_NONBLOCK flag */
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
+ goto out;
+ result = -ENXIO;
if (!scsi_block_when_processing_errors(sdp->device))
- return -ENXIO;
+ goto out;
+ result = -EFAULT;
if (!access_ok(VERIFY_WRITE, p, SZ_SG_IO_HDR))
- return -EFAULT;
- result =
- sg_new_write(sfp, p, SZ_SG_IO_HDR,
- blocking, read_only, &srp);
+ goto out;
+ result = sg_new_write(sfp, p, SZ_SG_IO_HDR, blocking,
+ read_only, &srp);
if (result < 0)
- return result;
+ goto out;
srp->sg_io_owned = 1;
while (1) {
result = 0; /* following macro to beat race condition */
__wait_event_interruptible(sfp->read_wait,
(sdp->detached || sfp->closed || sg_srp_done(srp, sfp)),
result);
- if (sdp->detached)
- return -ENODEV;
+ if (sdp->detached) {
+ result = -ENODEV;
+ goto out;
+ }
if (sfp->closed)
- return 0; /* request packet dropped already */
+ goto out; /* request packet dropped already */
if (0 == result)
break;
srp->orphan = 1;
- return result; /* -ERESTARTSYS because signal hit process */
+ goto out; /* -ERESTARTSYS because signal hit process */
}
write_lock_irqsave(&sfp->rq_list_lock, iflags);
srp->done = 2;
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
result = sg_new_read(sfp, p, SZ_SG_IO_HDR, srp);
- return (result < 0) ? result : 0;
+ if (result > 0)
+ result = 0;
+ goto out;
}
case SG_SET_TIMEOUT:
result = get_user(val, ip);
if (result)
- return result;
+ goto out;
+ result = -EIO;
if (val < 0)
- return -EIO;
- if (val >= MULDIV (INT_MAX, USER_HZ, HZ))
- val = MULDIV (INT_MAX, USER_HZ, HZ);
+ goto out;
+ if (val >= MULDIV(INT_MAX, USER_HZ, HZ))
+ val = MULDIV(INT_MAX, USER_HZ, HZ);
sfp->timeout_user = val;
sfp->timeout = MULDIV (val, HZ, USER_HZ);
- return 0;
+ result = 0;
+ goto out;
case SG_GET_TIMEOUT: /* N.B. User receives timeout as return value */
/* strange ..., for backward compatibility */
- return sfp->timeout_user;
+ result = sfp->timeout_user;
+ goto out;
case SG_SET_FORCE_LOW_DMA:
result = get_user(val, ip);
if (result)
- return result;
+ goto out;
if (val) {
sfp->low_dma = 1;
if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
@@ -846,21 +858,26 @@ sg_ioctl(struct inode *inode, struct file *filp,
sg_build_reserve(sfp, val);
}
} else {
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
+ goto out;
sfp->low_dma = sdp->device->host->unchecked_isa_dma;
}
- return 0;
+ result = 0;
+ goto out;
case SG_GET_LOW_DMA:
- return put_user((int) sfp->low_dma, ip);
+ result = put_user((int) sfp->low_dma, ip);
+ goto out;
case SG_GET_SCSI_ID:
+ result = -EFAULT;
if (!access_ok(VERIFY_WRITE, p, sizeof (sg_scsi_id_t)))
- return -EFAULT;
- else {
+ goto out;
+ {
sg_scsi_id_t __user *sg_idp = p;
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
+ goto out;
__put_user((int) sdp->device->host->host_no,
&sg_idp->host_no);
__put_user((int) sdp->device->channel,
@@ -874,29 +891,33 @@ sg_ioctl(struct inode *inode, struct file *filp,
&sg_idp->d_queue_depth);
__put_user(0, &sg_idp->unused[0]);
__put_user(0, &sg_idp->unused[1]);
- return 0;
+ result = 0;
+ goto out;
}
case SG_SET_FORCE_PACK_ID:
result = get_user(val, ip);
if (result)
- return result;
+ goto out;
sfp->force_packid = val ? 1 : 0;
- return 0;
+ result = 0;
+ goto out;
case SG_GET_PACK_ID:
+ result = -EFAULT;
if (!access_ok(VERIFY_WRITE, ip, sizeof (int)))
- return -EFAULT;
+ goto out;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
+ result = 0;
for (srp = sfp->headrp; srp; srp = srp->nextrp) {
if ((1 == srp->done) && (!srp->sg_io_owned)) {
read_unlock_irqrestore(&sfp->rq_list_lock,
iflags);
__put_user(srp->header.pack_id, ip);
- return 0;
+ goto out;
}
}
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
__put_user(-1, ip);
- return 0;
+ goto out;
case SG_GET_NUM_WAITING:
read_lock_irqsave(&sfp->rq_list_lock, iflags);
for (val = 0, srp = sfp->headrp; srp; srp = srp->nextrp) {
@@ -904,67 +925,76 @@ sg_ioctl(struct inode *inode, struct file *filp,
++val;
}
read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return put_user(val, ip);
+ result = put_user(val, ip);
+ goto out;
case SG_GET_SG_TABLESIZE:
- return put_user(sdp->sg_tablesize, ip);
+ result = put_user(sdp->sg_tablesize, ip);
+ goto out;
case SG_SET_RESERVED_SIZE:
result = get_user(val, ip);
if (result)
- return result;
- if (val < 0)
- return -EINVAL;
+ goto out;
+ result = -EINVAL;
+ if (val < 0)
+ goto out;
val = min_t(int, val,
sdp->device->request_queue->max_sectors * 512);
if (val != sfp->reserve.bufflen) {
+ result = -EBUSY;
if (sg_res_in_use(sfp) || sfp->mmap_called)
- return -EBUSY;
+ goto out;
sg_remove_scat(&sfp->reserve);
sg_build_reserve(sfp, val);
}
- return 0;
+ result = 0;
+ goto out;
case SG_GET_RESERVED_SIZE:
val = min_t(int, sfp->reserve.bufflen,
sdp->device->request_queue->max_sectors * 512);
- return put_user(val, ip);
+ result = put_user(val, ip);
+ goto out;
case SG_SET_COMMAND_Q:
result = get_user(val, ip);
- if (result)
- return result;
- sfp->cmd_q = val ? 1 : 0;
- return 0;
+ if (!result)
+ sfp->cmd_q = val ? 1 : 0;
+ goto out;
case SG_GET_COMMAND_Q:
- return put_user((int) sfp->cmd_q, ip);
+ result = put_user((int) sfp->cmd_q, ip);
+ goto out;
case SG_SET_KEEP_ORPHAN:
result = get_user(val, ip);
- if (result)
- return result;
- sfp->keep_orphan = val;
- return 0;
+ if (!result)
+ sfp->keep_orphan = val;
+ goto out;
case SG_GET_KEEP_ORPHAN:
- return put_user((int) sfp->keep_orphan, ip);
+ result = put_user((int) sfp->keep_orphan, ip);
+ goto out;
case SG_NEXT_CMD_LEN:
result = get_user(val, ip);
- if (result)
- return result;
- sfp->next_cmd_len = (val > 0) ? val : 0;
- return 0;
+ if (!result)
+ sfp->next_cmd_len = (val > 0) ? val : 0;
+ goto out;
case SG_GET_VERSION_NUM:
- return put_user(sg_version_num, ip);
+ result = put_user(sg_version_num, ip);
+ goto out;
case SG_GET_ACCESS_COUNT:
/* faked - we don't have a real access count anymore */
val = (sdp->device ? 1 : 0);
- return put_user(val, ip);
+ result = put_user(val, ip);
+ goto out;
case SG_GET_REQUEST_TABLE:
+ result = -EFAULT;
if (!access_ok(VERIFY_WRITE, p, SZ_SG_REQ_INFO * SG_MAX_QUEUE))
- return -EFAULT;
- else {
+ goto out;
+ {
sg_req_info_t *rinfo;
unsigned int ms;
rinfo = kmalloc(SZ_SG_REQ_INFO * SG_MAX_QUEUE,
GFP_KERNEL);
+ result = -ENOMEM;
if (!rinfo)
- return -ENOMEM;
+ goto out;
read_lock_irqsave(&sfp->rq_list_lock, iflags);
for (srp = sfp->headrp, val = 0; val < SG_MAX_QUEUE;
++val, srp = srp ? srp->nextrp : srp) {
@@ -998,25 +1028,29 @@ sg_ioctl(struct inode *inode, struct file *filp,
SZ_SG_REQ_INFO * SG_MAX_QUEUE);
result = result ? -EFAULT : 0;
kfree(rinfo);
- return result;
}
+ goto out;
case SG_EMULATED_HOST:
if (sdp->detached)
- return -ENODEV;
- return put_user(sdp->device->host->hostt->emulated, ip);
+ result = -ENODEV;
+ else
+ result = put_user(sdp->device->host->hostt->emulated, ip);
+ goto out;
case SG_SCSI_RESET:
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
+ goto out;
+ result = -EBUSY;
if (filp->f_flags & O_NONBLOCK) {
if (scsi_host_in_recovery(sdp->device->host))
- return -EBUSY;
+ goto out;
} else if (!scsi_block_when_processing_errors(sdp->device))
- return -EBUSY;
+ goto out;
result = get_user(val, ip);
if (result)
- return result;
+ goto out;
if (SG_SCSI_RESET_NOTHING == val)
- return 0;
+ goto out;
switch (val) {
case SG_SCSI_RESET_DEVICE:
val = SCSI_TRY_RESET_DEVICE;
@@ -1028,46 +1062,60 @@ sg_ioctl(struct inode *inode, struct file *filp,
val = SCSI_TRY_RESET_HOST;
break;
default:
- return -EINVAL;
+ result = -EINVAL;
+ goto out;
}
+ result = -EACCES;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
- return -EACCES;
- return (scsi_reset_provider(sdp->device, val) ==
- SUCCESS) ? 0 : -EIO;
+ goto out;
+ result = scsi_reset_provider(sdp->device, val) == SUCCESS?
+ 0 : -EIO;
+ goto out;
case SCSI_IOCTL_SEND_COMMAND:
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
+ goto out;
if (read_only) {
unsigned char opcode = WRITE_6;
Scsi_Ioctl_Command __user *siocp = p;
+ result = -EFAULT;
if (copy_from_user(&opcode, siocp->data, 1))
- return -EFAULT;
+ goto out;
+ result = -EPERM;
if (!sg_allow_access(opcode, sdp->device->type))
- return -EPERM;
+ goto out;
}
- return sg_scsi_ioctl(filp, sdp->device->request_queue, NULL, p);
+ result = sg_scsi_ioctl(filp, sdp->device->request_queue, NULL, p);
+ goto out;
case SG_SET_DEBUG:
result = get_user(val, ip);
- if (result)
- return result;
- sdp->sgdebug = (char) val;
- return 0;
+ if (!result)
+ sdp->sgdebug = (char) val;
+ goto out;
case SCSI_IOCTL_GET_IDLUN:
case SCSI_IOCTL_GET_BUS_NUMBER:
case SCSI_IOCTL_PROBE_HOST:
case SG_GET_TRANSFORM:
+ result = -ENODEV;
if (sdp->detached)
- return -ENODEV;
- return scsi_ioctl(sdp->device, cmd_in, p);
+ goto out;
+ result = scsi_ioctl(sdp->device, cmd_in, p);
+ goto out;
case BLKSECTGET:
- return put_user(sdp->device->request_queue->max_sectors * 512,
+ result = put_user(sdp->device->request_queue->max_sectors * 512,
ip);
+ goto out;
default:
+ result = -EPERM; /* don't know so take safe approach */
if (read_only)
- return -EPERM; /* don't know so take safe approach */
- return scsi_ioctl(sdp->device, cmd_in, p);
+ goto out;
+ result = scsi_ioctl(sdp->device, cmd_in, p);
+ goto out;
}
+out:
+ unlock_kernel();
+ return result;
}
#ifdef CONFIG_COMPAT
@@ -1314,7 +1362,7 @@ static struct file_operations sg_fops = {
.read = sg_read,
.write = sg_write,
.poll = sg_poll,
- .ioctl = sg_ioctl,
+ .unlocked_ioctl = sg_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = sg_compat_ioctl,
#endif
--
The only person who always got his work done by Friday was Robinson Crusoe
Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)
Powered by blists - more mailing lists