Re: [Xen-devel] [PATCH 3/8] libflask: Add boolean manipulation functions

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-02-02 at 14:28 +0000, Daniel De Graaf wrote:
> On 02/02/2012 04:06 AM, Ian Campbell wrote:
> > On Wed, 2012-02-01 at 19:09 +0000, Daniel De Graaf wrote:
> >> Add wrappers for getting and setting policy booleans by name or ID.
> >>
> >> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >> ---
> >>  tools/flask/libflask/flask_op.c         |   59 
> >> +++++++++++++++++++++++++++++++
> >>  tools/flask/libflask/include/libflask.h |    3 ++
> >>  2 files changed, 62 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/tools/flask/libflask/flask_op.c 
> >> b/tools/flask/libflask/flask_op.c
> >> index d4b8ef0..412a05d 100644
> >> --- a/tools/flask/libflask/flask_op.c
> >> +++ b/tools/flask/libflask/flask_op.c
> >> @@ -109,6 +109,65 @@ int flask_setenforce(xc_interface *xc_handle, int 
> >> mode)
> >>      return 0;
> >>  }
> >>  
> >> +int flask_getbool_byid(xc_interface *xc_handle, int id, char *name, int 
> >> *curr, int *pend)
> >> +{
> >> +    flask_op_t op;
> >> +    char buf[255];
> >> +    int rv;
> >> +
> >> +    op.cmd = FLASK_GETBOOL2;
> >> +    op.buf = buf;
> >> +    op.size = 255;
> > 
> > sizeof(buf)? Here and elsewhere (including a few existing locations in
> > flask_op.c).
> > 
> >> +
> >> +    snprintf(buf, sizeof buf, "%i", id);
> >> +
> >> +    rv = xc_flask_op(xc_handle, &op);
> >> +
> >> +    if ( rv )
> >> +        return rv;
> >> +    
> >> +    sscanf(buf, "%i %i %s", curr, pend, name);
> > 
> > Do you care about sscanf failures?
>  
> A failure here would be a sign of the hypervisor having made a format change
> that is not backwards compatible. Checking it would be more complete, however.
> 
> > It seems from other uses in the file that buf can contain binary data so
> > would it make sense to make this two ints as binary followed by a
> > string? That would remove string parsing here and in the hypervisor
> > (which seems more critical to me?)
> 
> That also seems far simpler to me; however, all the current FLASK hypercalls
> are done via string parsing so deviating from this for new operations would
> make them inconsistent.

OK. I thought I'd seen some binary muddling in their but I must have
been mistaken.

> If we didn't have to care about backwards compatibility I would convert the
> entire flask_op hypercall to use a union-of-structures similar to domctl
> because the string parsing introduces unneeded complexity.

How much do we care about backwards compat for this interface? Isn't it
a tools only dom0 interface?

> > Is there a defined maximum for the length of "name"?
> 
> INITCONTEXTLEN = 256.

So the max size of the buffer is 256 + whatever two int and two spaces
might maximally take, but your buffer is exactly 256.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel