[PATCH 0/2] xensore: fixing two bugs

[PATCH 0/2] xensore: fixing two bugs

[Juergen Gross]

Two small patches to fix bugs: one error path correction (setting
correct error code for caller) and add some missing checks for
allocation failures.

Juergen Gross (2):
  xenstore: set correct error code when violating quota
  xenstore: add missing checks for allocation failure

 tools/xenstore/xenstored_core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.10.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/2] xenstore: set correct error code when violating quota

[Juergen Gross]

When the number of permitted xenstore entries for a domain is being
exceeded the operation trying to create a new entry is denied.
Unfortunately errno isn't being set in this case so the error code
returned to the client is undefined.

Set errno to ENOSPC in this case.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
This is a backport candidate.
---
 tools/xenstore/xenstored_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 5c659d8..ed80345 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -937,8 +937,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
 	if (!parent)
 		return NULL;
 
-	if (domain_entry(conn) >= quota_nb_entry_per_domain)
+	if (domain_entry(conn) >= quota_nb_entry_per_domain) {
+		errno = ENOSPC;
 		return NULL;
+	}
 
 	/* Add child to parent. */
 	base = basename(name);
-- 
2.10.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/2] xenstore: add missing checks for allocation failure

[Juergen Gross]

Add a missing allocation failure checks.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index ed80345..fe11ff2 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -946,6 +946,8 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
 	base = basename(name);
 	baselen = strlen(base) + 1;
 	children = talloc_array(ctx, char, parent->childlen + baselen);
+	if (!children)
+		goto nomem;
 	memcpy(children, parent->children, parent->childlen);
 	memcpy(children + parent->childlen, base, baselen);
 	parent->children = children;
@@ -953,13 +955,19 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
 
 	/* Allocate node */
 	node = talloc(ctx, struct node);
+	if (!node)
+		goto nomem;
 	node->tdb = tdb_context(conn);
 	node->name = talloc_strdup(node, name);
+	if (!node->name)
+		goto nomem;
 
 	/* Inherit permissions, except unprivileged domains own what they create */
 	node->num_perms = parent->num_perms;
 	node->perms = talloc_memdup(node, parent->perms,
 				    node->num_perms * sizeof(node->perms[0]));
+	if (!node->perms)
+		goto nomem;
 	if (domain_is_unprivileged(conn))
 		node->perms[0].id = conn->id;
 
@@ -969,6 +977,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
 	node->parent = parent;
 	domain_entry_inc(conn, node);
 	return node;
+
+nomem:
+	errno = ENOMEM;
+	return NULL;
 }
 
 static int destroy_node(void *_node)
-- 
2.10.2


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] xenstore: set correct error code when violating quota

[Wei Liu]

On Mon, Mar 20, 2017 at 09:00:20AM +0100, Juergen Gross wrote:
> When the number of permitted xenstore entries for a domain is being
> exceeded the operation trying to create a new entry is denied.
> Unfortunately errno isn't being set in this case so the error code
> returned to the client is undefined.
> 
> Set errno to ENOSPC in this case.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

But ...

> ---
> This is a backport candidate.
> ---
>  tools/xenstore/xenstored_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 5c659d8..ed80345 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -937,8 +937,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
>  	if (!parent)
>  		return NULL;

What about this and other places?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] xenstore: set correct error code when violating quota

[Wei Liu]

On Tue, Mar 21, 2017 at 05:14:47PM +0000, Wei Liu wrote:
> On Mon, Mar 20, 2017 at 09:00:20AM +0100, Juergen Gross wrote:
> > When the number of permitted xenstore entries for a domain is being
> > exceeded the operation trying to create a new entry is denied.
> > Unfortunately errno isn't being set in this case so the error code
> > returned to the client is undefined.
> > 
> > Set errno to ENOSPC in this case.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> But ...
> 
> > ---
> > This is a backport candidate.
> > ---
> >  tools/xenstore/xenstored_core.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> > index 5c659d8..ed80345 100644
> > --- a/tools/xenstore/xenstored_core.c
> > +++ b/tools/xenstore/xenstored_core.c
> > @@ -937,8 +937,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
> >  	if (!parent)
> >  		return NULL;
> 
> What about this and other places?

Oh, this relies on errno being set in some other place(s).

This is really poor error handling style... :-/

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] xenstore: add missing checks for allocation failure

[Wei Liu]

On Mon, Mar 20, 2017 at 09:00:21AM +0100, Juergen Gross wrote:
> Add a missing allocation failure checks.

s/a//

> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] xenstore: set correct error code when violating quota

[Juergen Gross]

On 21/03/17 18:16, Wei Liu wrote:
> On Tue, Mar 21, 2017 at 05:14:47PM +0000, Wei Liu wrote:
>> On Mon, Mar 20, 2017 at 09:00:20AM +0100, Juergen Gross wrote:
>>> When the number of permitted xenstore entries for a domain is being
>>> exceeded the operation trying to create a new entry is denied.
>>> Unfortunately errno isn't being set in this case so the error code
>>> returned to the client is undefined.
>>>
>>> Set errno to ENOSPC in this case.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> But ...
>>
>>> ---
>>> This is a backport candidate.
>>> ---
>>>  tools/xenstore/xenstored_core.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
>>> index 5c659d8..ed80345 100644
>>> --- a/tools/xenstore/xenstored_core.c
>>> +++ b/tools/xenstore/xenstored_core.c
>>> @@ -937,8 +937,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
>>>  	if (!parent)
>>>  		return NULL;
>>
>> What about this and other places?
> 
> Oh, this relies on errno being set in some other place(s).
> 
> This is really poor error handling style... :-/

I can fix this up in another patch, but I don't think this should
be done in this patch (and possibly not in 4.9).


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] xenstore: set correct error code when violating quota

[Wei Liu]

On Tue, Mar 21, 2017 at 06:21:24PM +0100, Juergen Gross wrote:
> On 21/03/17 18:16, Wei Liu wrote:
> > On Tue, Mar 21, 2017 at 05:14:47PM +0000, Wei Liu wrote:
> >> On Mon, Mar 20, 2017 at 09:00:20AM +0100, Juergen Gross wrote:
> >>> When the number of permitted xenstore entries for a domain is being
> >>> exceeded the operation trying to create a new entry is denied.
> >>> Unfortunately errno isn't being set in this case so the error code
> >>> returned to the client is undefined.
> >>>
> >>> Set errno to ENOSPC in this case.
> >>>
> >>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>
> >> Acked-by: Wei Liu <wei.liu2@citrix.com>
> >>
> >> But ...
> >>
> >>> ---
> >>> This is a backport candidate.
> >>> ---
> >>>  tools/xenstore/xenstored_core.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> >>> index 5c659d8..ed80345 100644
> >>> --- a/tools/xenstore/xenstored_core.c
> >>> +++ b/tools/xenstore/xenstored_core.c
> >>> @@ -937,8 +937,10 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
> >>>  	if (!parent)
> >>>  		return NULL;
> >>
> >> What about this and other places?
> > 
> > Oh, this relies on errno being set in some other place(s).
> > 
> > This is really poor error handling style... :-/
> 
> I can fix this up in another patch, but I don't think this should
> be done in this patch (and possibly not in 4.9).
> 

Of course. I didn't mean to ask you fix them in one patch.

> 
> Juergen
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 0/2] xensore: fixing two bugs

[Ian Jackson]

Juergen Gross writes ("[PATCH 0/2] xensore: fixing two bugs"):
> Two small patches to fix bugs: one error path correction (setting
> correct error code for caller) and add some missing checks for
> allocation failures.
> 
> Juergen Gross (2):
>   xenstore: set correct error code when violating quota
>   xenstore: add missing checks for allocation failure

Both,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel