Re: [Xen-devel] [PATCH v4 00/12] xenstore: support reading directory with many children

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/12/16 12:05, Wei Liu wrote:

On Mon, Dec 05, 2016 at 08:48:41AM +0100, Juergen Gross wrote:
[...]

Juergen Gross (12):
  xenstore: modify add_change_node() parameter types
  xenstore: call add_change_node() directly when writing node
  xenstore: use common tdb record header in xenstore
  xenstore: add per-node generation counter
  xenstore: add support for reading directory with many children
  xenstore: support XS_DIRECTORY_PART in libxenstore
  xenstore: use array for xenstore wire command handling
  xenstore: let command functions return error or success
  xenstore: make functions static
  xenstore: add helper functions for wire argument parsing
  xenstore: add small default data buffer to internal struct
  xenstore: handle memory allocation failures in xenstored

Applied to staging.

XenServer's Coverity has run, and has a few things to say.  Its not obvious (i.e. I can't trivially identify) if these are preexisting bugs which your code has brought to light, or introduced by your series.

Both do_rm() and do_mkdir() suffer from the same problem.

onearg(in) may return NULL, which results in get_node_canonicalized() setting name to NULL and returning NULL.  name is then dereferenced in the error path by get_parent()/create_node() respectively.

There are two complains of leaking a talloc_array() allocation.  First in do_set_perms() via the xs_strings_to_perms() failure path and second in send_directory_part() via the success path.  The use of talloc_array() in add_event() would suggest that you are expected to call talloc_free() on the result, unless I am missing something subtle.

There is another resource leak complaint of bdata in send_reply(), although this case clearly hasn't observed the list_add_tail(), so I don't think this is a real issue.

Finally, xs_directory_part(), on line 619 uses a strncpy() for all bytes of gen, but doesn't necesserily NULL terminate it, and is later used by strcmp().

~Andrew

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