Re: [PATCH v2] block: register_blkdev doesn't check name against NULL
On 09/29/2013 02:55 PM, vaughan wrote: On 09/23/2013 10:56 PM, Jeff Moyer wrote: Vaughan Cao writes: register_blkdev(0, NULL) can result kernel Oops by copying from NULL in strlcpy(). Fix it by checking NULL pointer at the beginning and WARN when encountered in unregister_blkdev. Uhh, so yeah, this is an exported function, so I could see maybe wanting to do the argument checking. But honestly, if your driver can't even get this right, is there any hope of it actually working? This seems like a pointless patch to me, but ultimately it's up to Jens. Cheers, Jeff p.s. the kerneldoc tells you what to put there: * @name: the name of the new block device as a zero terminated string Thanks for your comment, Jeff. I have the same feeling as you, however, shouldn't kernel do its best to provide the maximum stable working ability? And it's test case 7 of block driver in ltp project - http://sourceforge.net/p/ltp/git/ci/master/tree/testcases/kernel/device-drivers/block/kernel_space/test_block.c. It seems their attitude is we should check this. I checked most of the callers of this function in current code tree. Indeed, mostly they pass a static string as a parameter. As jeff has said if a driver wants to get things right, it should able to give right parameters. But as an acknowledge of kernel code protocol, I have a query/question of the similar situation. if a function is a public one and called rather commonly. In what cases should we have a parameter checking in it? I think if the parameter is a rather obvious one that even a look at it in the caller telling OK or not. These parameters may not need checking. thanks chai wen Vaughan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] block: register_blkdev doesn't check name against NULL
On 09/29/2013 02:55 PM, vaughan wrote: On 09/23/2013 10:56 PM, Jeff Moyer wrote: Vaughan Caovaughan@oracle.com writes: register_blkdev(0, NULL) can result kernel Oops by copying from NULL in strlcpy(). Fix it by checking NULL pointer at the beginning and WARN when encountered in unregister_blkdev. Uhh, so yeah, this is an exported function, so I could see maybe wanting to do the argument checking. But honestly, if your driver can't even get this right, is there any hope of it actually working? This seems like a pointless patch to me, but ultimately it's up to Jens. Cheers, Jeff p.s. the kerneldoc tells you what to put there: * @name: the name of the new block device as a zero terminated string Thanks for your comment, Jeff. I have the same feeling as you, however, shouldn't kernel do its best to provide the maximum stable working ability? And it's test case 7 of block driver in ltp project - http://sourceforge.net/p/ltp/git/ci/master/tree/testcases/kernel/device-drivers/block/kernel_space/test_block.c. It seems their attitude is we should check this. I checked most of the callers of this function in current code tree. Indeed, mostly they pass a static string as a parameter. As jeff has said if a driver wants to get things right, it should able to give right parameters. But as an acknowledge of kernel code protocol, I have a query/question of the similar situation. if a function is a public one and called rather commonly. In what cases should we have a parameter checking in it? I think if the parameter is a rather obvious one that even a look at it in the caller telling OK or not. These parameters may not need checking. thanks chai wen Vaughan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] brd: return register_blkdev err code when it fails
Hi Nick
when register_blk fails,no matter what reasons,brd_init always
returns a "-EIO" error code. if brd_init returns the error code
returned by register_blkdev,the formatted error info reported
when module inserting failed(caused by register_blkdev failing)
could be more meaningfull.
Signed-off-by: chaiwen
---
drivers/block/brd.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 9bf4371..361204d 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -557,6 +557,7 @@ static int __init brd_init(void)
int i, nr;
unsigned long range;
struct brd_device *brd, *next;
+ int ret;
/*
* brd module now has a feature to instantiate underlying device
@@ -602,8 +603,8 @@ static int __init brd_init(void)
range = 1UL << MINORBITS;
}
- if (register_blkdev(RAMDISK_MAJOR, "ramdisk"))
- return -EIO;
+ if (ret = register_blkdev(RAMDISK_MAJOR, "ramdisk"))
+ return ret;
for (i = 0; i < nr; i++) {
brd = brd_alloc(i);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] brd: return register_blkdev err code when it fails
Hi Nick
when register_blk fails,no matter what reasons,brd_init always
returns a -EIO error code. if brd_init returns the error code
returned by register_blkdev,the formatted error info reported
when module inserting failed(caused by register_blkdev failing)
could be more meaningfull.
Signed-off-by: chaiwen chaiw.f...@cn.fujitsu.com
---
drivers/block/brd.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 9bf4371..361204d 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -557,6 +557,7 @@ static int __init brd_init(void)
int i, nr;
unsigned long range;
struct brd_device *brd, *next;
+ int ret;
/*
* brd module now has a feature to instantiate underlying device
@@ -602,8 +603,8 @@ static int __init brd_init(void)
range = 1UL MINORBITS;
}
- if (register_blkdev(RAMDISK_MAJOR, ramdisk))
- return -EIO;
+ if (ret = register_blkdev(RAMDISK_MAJOR, ramdisk))
+ return ret;
for (i = 0; i nr; i++) {
brd = brd_alloc(i);
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
