All of lore.kernel.org
 help / color / mirror / Atom feed
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/
>


      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.