Re: [PATCH] tools/xenstored: Don't assume errno will not be overwritten in lu_arch()

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 30/07/2021 09:40, Juergen Gross wrote:
> On 29.07.21 17:23, Julien Grall wrote:
> > On 29/07/2021 12:06, Julien Grall wrote:
> > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > >
> > > At the moment, do_control_lu() will set errno to 0 before calling
> > > lu_arch() and then check errno. The expectation is nothing in lu_arch()
> > > will change the value unless there is an error.
> > >
> > > However, per errno(3), a function that succeeds is allowed to change
> > > errno. In fact, syslog() will overwrite errno if the logs are rotated
> > > at the time it is called.
> > >
> > > To prevent any further issue, errno is now always set before
> > > returning NULL.
> > >
> > > Additionally, errno is only checked when returning NULL so the client
> > > can see the error message if there is any.
> > >
> > > Reported-by: Michael Kurth <mku@xxxxxxxxxx>
> > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > > ---
> > >   tools/xenstore/xenstored_control.c | 5 +++--
> > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/xenstore/xenstored_control.c 
> > > b/tools/xenstore/xenstored_control.c
> > > index 6b68b79faac7..6fcb42095b59 100644
> > > --- a/tools/xenstore/xenstored_control.c
> > > +++ b/tools/xenstore/xenstored_control.c
> > > @@ -324,6 +324,7 @@ static const char *lu_binary_alloc(const void 
> > > *ctx, struct connection *conn,
> > >       lu_status->kernel_size = size;
> > >       lu_status->kernel_off = 0;
> > > +    errno = 0;
> > >       return NULL;
> > >   }
> > > @@ -339,6 +340,7 @@ static const char *lu_binary_save(const void 
> > > *ctx, struct connection *conn,
> > >       memcpy(lu_status->kernel + lu_status->kernel_off, data, size);
> > >       lu_status->kernel_off += size;
> > > +    errno = 0;
> > >       return NULL;
> > >   }
> > I forgot to update lu_binary(). I will respin the patch once I get 
> > some feedback.
> With setting errno to 0 before returning NULL in lu_binary() you can add
>
> Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
Thanks! I will commit it.

Cheers,

--
Julien Grall