Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Markus Armbruster 
Philippe Mathieu-Daudé  writes:

> On 03/02/2017 08:32 PM, Eric Blake wrote:
>> On 03/02/2017 03:43 PM, Markus Armbruster wrote:
>>> sd_parse_uri() truncates long VDI names silently.  Reject them
>>> instead.
>>>
>>> Signed-off-by: Markus Armbruster 
>>> ---
>>>  block/sheepdog.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>>> index deb110e..72a52a6 100644
>>> --- a/block/sheepdog.c
>>> +++ b/block/sheepdog.c
>>> @@ -985,7 +985,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const 
>>> char *filename,
>>>  ret = -EINVAL;
>>>  goto out;
>>>  }
>>> -pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1);
>>> +if (g_strlcpy(vdi, uri->path + 1, SD_MAX_VDI_LEN) >= SD_MAX_VDI_LEN) {
>>> +goto out;
>>> +}
>>
>> Does this need to set ret? Maybe to -EINVAL?
>>
>
> ups I missed that. what about -ENAMETOOLONG?
> bdrv callers seem to only test for 'ret < 0'.

The next patch gets rid of the error code in this function.

>>>
>>>  qp = query_params_parse(uri->query);
>>>  if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
>>>
>>

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Markus Armbruster 
Eric Blake  writes:

> On 03/02/2017 03:43 PM, Markus Armbruster wrote:
>> sd_parse_uri() truncates long VDI names silently.  Reject them
>> instead.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/sheepdog.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index deb110e..72a52a6 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -985,7 +985,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const char 
>> *filename,
>>  ret = -EINVAL;
>>  goto out;
>>  }
>> -pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1);
>> +if (g_strlcpy(vdi, uri->path + 1, SD_MAX_VDI_LEN) >= SD_MAX_VDI_LEN) {
>> +goto out;
>> +}
>
> Does this need to set ret? Maybe to -EINVAL?

Yes.  The next patch heals it, but of course I'll fix it anyway.

>>  
>>  qp = query_params_parse(uri->query);
>>  if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
>>

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Philippe Mathieu-Daudé 

On 03/02/2017 08:32 PM, Eric Blake wrote:

On 03/02/2017 03:43 PM, Markus Armbruster wrote:

sd_parse_uri() truncates long VDI names silently.  Reject them
instead.

Signed-off-by: Markus Armbruster 
---
 block/sheepdog.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index deb110e..72a52a6 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -985,7 +985,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const char 
*filename,
 ret = -EINVAL;
 goto out;
 }
-pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1);
+if (g_strlcpy(vdi, uri->path + 1, SD_MAX_VDI_LEN) >= SD_MAX_VDI_LEN) {
+goto out;
+}


Does this need to set ret? Maybe to -EINVAL?



ups I missed that. what about -ENAMETOOLONG?
bdrv callers seem to only test for 'ret < 0'.



 qp = query_params_parse(uri->query);
 if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Philippe Mathieu-Daudé 

On 03/02/2017 06:43 PM, Markus Armbruster wrote:

sd_parse_uri() truncates long VDI names silently.  Reject them
instead.

Signed-off-by: Markus Armbruster 


Reviewed-by: Philippe Mathieu-Daudé 


---
 block/sheepdog.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index deb110e..72a52a6 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -985,7 +985,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const char 
*filename,
 ret = -EINVAL;
 goto out;
 }
-pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1);
+if (g_strlcpy(vdi, uri->path + 1, SD_MAX_VDI_LEN) >= SD_MAX_VDI_LEN) {
+goto out;
+}

 qp = query_params_parse(uri->query);
 if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {

Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create()

2017-03-02 Thread Eric Blake 
On 03/02/2017 03:43 PM, Markus Armbruster wrote:
> sd_parse_uri() truncates long VDI names silently.  Reject them
> instead.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  block/sheepdog.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index deb110e..72a52a6 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -985,7 +985,9 @@ static int sd_parse_uri(BDRVSheepdogState *s, const char 
> *filename,
>  ret = -EINVAL;
>  goto out;
>  }
> -pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1);
> +if (g_strlcpy(vdi, uri->path + 1, SD_MAX_VDI_LEN) >= SD_MAX_VDI_LEN) {
> +goto out;
> +}

Does this need to set ret? Maybe to -EINVAL?

>  
>  qp = query_params_parse(uri->query);
>  if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature