On 11/16/2017 03:35 AM, Wouter Verhelst wrote:
> Hi Eric,
>
> On Wed, Nov 15, 2017 at 03:08:05PM -0600, Eric Blake wrote:
>> We had a couple of bugs when dealing with bogus requests from
>> malicious clients,
>
> You had me going here for a while. Upon seeing "malicious", my mind
> screamed "security issue". Good to know it isn't that :-)
I'll amend the commit message to state that despite the client's attempt
to violate protocol, we are still not crashing and thus this is not
security (just a matter of quality of implementation).
>> + /* Caller did range checking */
>
> Might be good to have an assert here, still, so that future changes
> don't break things?
>
> Not critical though.
Easy enough to add.
>
>> /* For copy-on-write, we should trim on the diff file. Not yet
>> * implemented. */
>> if(client->server->flags & F_COPYONWRITE) {
>> diff --git a/tests/code/trim.c b/tests/code/trim.c
>> index 11f5ddf..01588e0 100644
>> --- a/tests/code/trim.c
>> +++ b/tests/code/trim.c
>> @@ -71,10 +71,5 @@ int main(void) {
>> count_assert(g_off == 0);
>> count_assert(g_len == 1024*1024);
>>
>> - req.from = 1024 * 1024 * 1024;
>> - req.len = 1024 * 1024;
>> - count_assert(exptrim(&req, &cl) == -1);
>> - count_assert(errno == EINVAL);
>> -
>
> Sightly sad that this reduces our test coverage, but I don't see a
> better way (other than moving more stuff to nbdsrv.c, which is my
> long-term goal anyway, but not something that I think I should ask for
> such a minor change).
>
Okay, will push the slightly-amended patch shortly.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature