From: chaiwen <chaiw.fnst@cn.fujitsu.com>
To: vaughan <vaughan.cao@oracle.com>
Cc: Jeff Moyer <jmoyer@redhat.com>,
axboe@kernel.dk, linux-kernel@vger.kernel.org,
alexey.kodanev@oracle.com
Subject: Re: [PATCH v2] block: register_blkdev doesn't check name against NULL
Date: Mon, 30 Sep 2013 10:19:52 +0800 [thread overview]
Message-ID: <5248DFC8.7060800@cn.fujitsu.com> (raw)
In-Reply-To: <5247CECC.9050003@oracle.com>
On 09/29/2013 02:55 PM, vaughan wrote:
> On 09/23/2013 10:56 PM, Jeff Moyer wrote:
>> Vaughan Cao<vaughan.cao@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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
prev parent reply other threads:[~2013-09-30 2:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-10 10:40 [PATCH] block: register_blkdev doesn't check name against NULL Vaughan Cao
2013-09-11 2:24 ` [PATCH v2] " Vaughan Cao
2013-09-23 14:56 ` Jeff Moyer
2013-09-29 6:55 ` vaughan
2013-09-30 2:19 ` chaiwen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5248DFC8.7060800@cn.fujitsu.com \
--to=chaiw.fnst@cn.fujitsu.com \
--cc=alexey.kodanev@oracle.com \
--cc=axboe@kernel.dk \
--cc=jmoyer@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=vaughan.cao@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.