[PATCH 11/13] libgpo: replace dup_sec_desc() usage

Richard Sharpe realrichardsharpe at gmail.com
Mon May 26 14:20:48 MDT 2014


On Mon, May 26, 2014 at 12:59 PM, Volker Lendecke
<Volker.Lendecke at sernet.de> wrote:
> On Mon, May 26, 2014 at 05:30:01PM +0200, David Disseldorp wrote:
>> Use security_descriptor_copy() instead, which is also provided by
>> libcli.
>>
>> Signed-off-by: David Disseldorp <ddiss at samba.org>
>> ---
>>  libgpo/gpo_util.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/libgpo/gpo_util.c b/libgpo/gpo_util.c
>> index 5b801c4..e90b9a3 100644
>> --- a/libgpo/gpo_util.c
>> +++ b/libgpo/gpo_util.c
>> @@ -773,7 +773,13 @@ NTSTATUS gpo_copy(TALLOC_CTX *mem_ctx,
>>               }
>>       }
>>
>> -     gpo->security_descriptor = dup_sec_desc(gpo, gpo_src->security_descriptor);
>> +     if (gpo_src->security_descriptor == NULL) {
>> +             /* existing SD assumed */
>> +             TALLOC_FREE(gpo);
>> +             return NT_STATUS_INVALID_PARAMETER;
>> +     }
>> +     gpo->security_descriptor = security_descriptor_copy(gpo,
>> +                                             gpo_src->security_descriptor);
>>       if (gpo->security_descriptor == NULL) {
>>               TALLOC_FREE(gpo);
>>               return NT_STATUS_NO_MEMORY;
>
> Not sure it matters, but this looks like a little semantic
> change in case gpo_src->security_descriptor==NULL. Before
> the patch we return INVALID_PARAMETER, after the patch I
> believe we crash.

Did you misread the patch? Looks like there is a test for
gpo->security_descriptor and a return if NULL before it is
dereferenced.

-- 
Regards,
Richard Sharpe
(何以解憂?唯有杜康。--曹操)


More information about the samba-technical mailing list