[PATCH] sharesec: Remove error message for unmarshall_sec_desc failure

Christof Schmitt cs at samba.org
Mon Jul 6 20:38:08 CEST 2015


On Mon, Jul 06, 2015 at 10:55:17AM -0700, Jeremy Allison wrote:
> On Fri, Jul 03, 2015 at 06:49:10AM +0200, Volker Lendecke wrote:
> > On Thu, Jul 02, 2015 at 12:49:19PM -0700, Christof Schmitt wrote:
> > > Just an additional remark: unmarshall_sec_desc still prints an error, in
> > > case the tdb entry is not empty, but cannot be parsed:
> > 
> > This is about ctdb not treating 0-length and nonexisting
> > records differently, right? My opinion here is that we
> > should not call unmarshall_security_descriptor from
> > get_share_security() if the record is 0-sized and not
> > suppress the error message if it failed.
> > 
> > You patch certainly achieves something similar, so that's
> > just my taste.

unmarshall_sec_desc already does everything that we need here: It
returns false on an empty record which triggers using the default, and
it also warns in the case of a non-empty record that has invalid ndr
data. We can add another check in get_share_security, but that would be
duplicated effort.

> I think Christof's patch is ok, as it does
> push any record (whether zero length or not)
> through unmarshall_security_descriptor which
> would be the right thing to do if ctdb does
> end up treating 0-length and nonexisting
> records differently at some point.
> 
> So just a quick question, why does ctdb
> return a zero-length record in this case ?

Amitay can probably better explain the details. Basically deleting a
record through ctdb means storing a record with empty data.  There is no
distinction made currently between empty and deleted records.  There was
a code path in dbwrap_ctdb that reported empty records as non-existing,
but that got removed in 925625b52, and that triggers the current problem
with the false warning.

Christof


More information about the samba-technical mailing list