From: Stoyan Gaydarov <[email protected]>
Signed-off-by: Stoyan Gaydarov <[email protected]>
---
drivers/scsi/osst.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 0ea78d9..80e7e98 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)
/* The ioctl command */
-static int osst_ioctl(struct inode * inode,struct file * file,
- unsigned int cmd_in, unsigned long arg)
+static long osst_ioctl(struct file * file, unsigned int cmd_in,
+ unsigned long arg)
{
+ lock_kernel();
int i, cmd_nr, cmd_type, blk, retval = 0;
struct st_modedef * STm;
struct st_partstat * STps;
@@ -4867,8 +4868,10 @@ static int osst_ioctl(struct inode * inode,struct file * file,
char * name = tape_name(STp);
void __user * p = (void __user *)arg;
- if (mutex_lock_interruptible(&STp->lock))
+ if (mutex_lock_interruptible(&STp->lock)) {
+ unlock_kernel();
return -ERESTARTSYS;
+ }
#if DEBUG
if (debugging && !STp->in_use) {
@@ -5187,6 +5190,7 @@ out:
mutex_unlock(&STp->lock);
+ unlock_kernel();
return retval;
}
@@ -5542,7 +5546,7 @@ static const struct file_operations osst_fops = {
.owner = THIS_MODULE,
.read = osst_read,
.write = osst_write,
- .ioctl = osst_ioctl,
+ .unlocked_ioctl = osst_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = osst_compat_ioctl,
#endif
--
1.6.2
On Tue, 2009-03-24 at 16:12 -0500, [email protected] wrote:
> From: Stoyan Gaydarov <[email protected]>
>
> Signed-off-by: Stoyan Gaydarov <[email protected]>
> ---
> drivers/scsi/osst.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
> index 0ea78d9..80e7e98 100644
> --- a/drivers/scsi/osst.c
> +++ b/drivers/scsi/osst.c
> @@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)
>
>
> /* The ioctl command */
> -static int osst_ioctl(struct inode * inode,struct file * file,
> - unsigned int cmd_in, unsigned long arg)
> +static long osst_ioctl(struct file * file, unsigned int cmd_in,
> + unsigned long arg)
> {
> + lock_kernel();
What necessitates the kernel locking? When st was audited, it didn't
need it; since osst is in many ways a copy of st, I'm surprised it does.
James
On Tue, Mar 24, 2009 at 4:24 PM, James Bottomley
<[email protected]> wrote:
> On Tue, 2009-03-24 at 16:12 -0500, [email protected] wrote:
>> From: Stoyan Gaydarov <[email protected]>
>>
>> Signed-off-by: Stoyan Gaydarov <[email protected]>
>> ---
>> drivers/scsi/osst.c | 12 ++++++++----
>> 1 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
>> index 0ea78d9..80e7e98 100644
>> --- a/drivers/scsi/osst.c
>> +++ b/drivers/scsi/osst.c
>> @@ -4856,9 +4856,10 @@ static int os_scsi_tape_close(struct inode * inode, struct file * filp)
>>
>>
>> /* The ioctl command */
>> -static int osst_ioctl(struct inode * inode,struct file * file,
>> - unsigned int cmd_in, unsigned long arg)
>> +static long osst_ioctl(struct file * file, unsigned int cmd_in,
>> + unsigned long arg)
>> {
>> + lock_kernel();
>
> What necessitates the kernel locking? When st was audited, it didn't
> need it; since osst is in many ways a copy of st, I'm surprised it does.
>
> James
The lock was there before, because it is used before calling
osst_ioctl, my patches don't change any functionality. It is meant to
show the maintainers of the code that the lock is there and to allow
them make a decision as to how best to handle the code without the
kernel locking. If it is not needed then the locks can be removed and
the function call can remain under the unlocked_ioctl.
-Stoyan
>
>
>
--
-Stoyan
On Tue, Mar 24, 2009 at 04:12:41PM -0500, [email protected] wrote:
> +static long osst_ioctl(struct file * file, unsigned int cmd_in,
> + unsigned long arg)
> {
> + lock_kernel();
> int i, cmd_nr, cmd_type, blk, retval = 0;
Did you try compiling this? If so, what compiler did you use? You
*should* have got an error about mixing code and declarations.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."