* Re: [PATCH] fix vulnerability of the release method of file operations in Block layer SCSI generic driver
2010-11-10 15:18 ` Matthew Wilcox
@ 2010-11-11 13:43 ` Hillf Danton
2010-12-09 13:49 ` Hillf Danton
1 sibling, 0 replies; 4+ messages in thread
From: Hillf Danton @ 2010-11-11 13:43 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-kernel, linux-scsi, Zou, Yi, Joe Eykholt
On Wed, Nov 10, 2010 at 11:18 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, Nov 10, 2010 at 10:08:37PM +0800, Hillf Danton wrote:
>> The computation context setup by previous opening the bsg file could
>> not survive following open/release operations upon the same file
>> object.
>
> Umm .. release is called on final close of a file, not on every close
> of a file.
>
right, thanks//Hillf
>> The vulnerability is fixed by deferring the cleanup operation until necessary.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>> --- a/block/bsg.c 2010-09-13 07:07:38.000000000 +0800
>> +++ b/block/bsg.c 2010-11-10 21:43:58.000000000 +0800
>> @@ -858,7 +858,8 @@ static int bsg_release(struct inode *ino
>> {
>> struct bsg_device *bd = file->private_data;
>>
>> - file->private_data = NULL;
>> + if (1 == atomic_read(&bd->ref_count))
>> + file->private_data = NULL;
>> return bsg_put_device(bd);
>> }
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> 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."
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fix vulnerability of the release method of file operations in Block layer SCSI generic driver
2010-11-10 15:18 ` Matthew Wilcox
2010-11-11 13:43 ` Hillf Danton
@ 2010-12-09 13:49 ` Hillf Danton
1 sibling, 0 replies; 4+ messages in thread
From: Hillf Danton @ 2010-12-09 13:49 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: linux-kernel, linux-scsi, Zou, Yi, Joe Eykholt
On Wed, Nov 10, 2010 at 11:18 PM, Matthew Wilcox <matthew@wil.cx> wrote:
> On Wed, Nov 10, 2010 at 10:08:37PM +0800, Hillf Danton wrote:
>> The computation context setup by previous opening the bsg file could
>> not survive following open/release operations upon the same file
>> object.
>
> Umm .. release is called on final close of a file, not on every close
> of a file.
Thanks for sharing the knowledge about the release callback.
But I am still wondering, if the operations in the release is safe, how the
increments, pumped up by open operations on cmdline, of the ref_count
of bsg device then get decreased, which could trigger mm leakage.
And both vulnerability and leakage, if possible, could be fixed by replacing
the only line we concerned with printk, see below please.
Cheers
Hillf
---
--- a/block/bsg.c 2010-11-01 19:54:12.000000000 +0800
+++ b/block/bsg.c 2010-12-09 21:38:32.000000000 +0800
@@ -855,7 +855,10 @@ static int bsg_release(struct inode *ino
{
struct bsg_device *bd = file->private_data;
- file->private_data = NULL;
+ if (atomic_read(&bd->ref_count) != 1)
+ printk(KERN_WARNING "bsg: ref count of %s is %d\n",
+ bd->name, atomic_read(&bd->ref_count));
+
return bsg_put_device(bd);
}
^ permalink raw reply [flat|nested] 4+ messages in thread