Re: [PATCH 1/2] scsi: scsi_debug: fix some bugs in sdebug_error_write()

[Dan Carpenter <dan.carpenter@linaro.org>]

On Wed, Oct 25, 2023 at 02:10:41PM +0800, Wenchao Hao wrote:
> On 2023/10/25 12:11, Dan Carpenter wrote:
> > On Wed, Oct 25, 2023 at 01:09:34AM +0800, Wenchao Hao wrote:
> > > Yes, there is bug here if write with .c code. Because your change to use
> > > strndup_user() would make write with dirty data appended to "ubuf" failed,
> > 
> > I don't understand this sentence.  What is "dirty" data in this context?
> > 
> 
> This is what I posted in previous reply:
> 
> We might have following pairs of parameters for sdebug_error_write:
> 
> ubuf: "0 -10 0x12\n0 0 0x2 0x6 0x4 0x2"
> count=11
> 
> the valid data in ubuf is "0 -10 -x12\n", others are dirty data.
> strndup_user() would return EINVAL for this pair which caused
> a correct write to fail.

I mean, I could just tell you that your kzalloc(count + 1, GFP_KERNEL)
fix will work.  It does work.

But how is passing "dirty data" a "correct write"?  I feel like it
should be treated as incorrect and returning -EINVAL is the correct
behavior.  I'm so confused.  Why are users doing that?

I have looked at the code and it just doesn't seem that complicated.
They shouldn't be passing messed up strings and expect the kernel to
allow it.

regards,
dan carpenter