Re: libsemanage patch

[Joshua Brindle <method@manicmethod.com>]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]



Joshua Brindle wrote:
>
>
> Chad Sellers wrote:
>> On 9/7/09 6:44 AM, "Daniel J Walsh"<dwalsh@redhat.com>  wrote:
>>
>>    
>>> On 09/04/2009 09:56 AM, Joshua Brindle wrote:
>>>      
>>>> Daniel J Walsh wrote:
>>>>        
>>>>> If you have a homedir that ends in '/', genhomedircon gets confused.
>>>>>
>>>>> # useradd -h /home2/dwalsh/ dwalsh
>>>>> # genhomedircon
>>>>>
>>>>> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel
>>>>> home root.
>>>>>
>>>>> We should just get rid of this command...  :^)
>>>>>
>>>>> Patch removes all trailing '/' from homedir.
>>>>>          
>>>>> diff --exclude-from=exclude -N -u -r
>>>>> nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
>>>>> --- nsalibsemanage/src/genhomedircon.c    2008-08-28
>>>>> 09:34:24.000000000 -0400
>>>>> +++ libsemanage-2.0.33/src/genhomedircon.c    2009-07-15
>>>>> 10:32:20.000000000 -0400
>>>>> @@ -304,6 +304,10 @@
>>>>>               continue;
>>>>>           if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>               continue;
>>>>> +        int len = strlen(pwbuf->pw_dir) -1;
>>>>> +        for(; len>  0&&  pwbuf->pw_dir[len]=='/'; len--) {
>>>>> +            pwbuf->pw_dir[len]=0;
>>>>> +        }
>>>>>           if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>>               continue;
>>>>>           if (semanage_str_count(pwbuf->pw_dir, '/')<= 1)
>>>>>          
>>>> Why aren't you just doing:
>>>>
>>>> len = strlen(pwbuf->pwdir);
>>>> if (pwbuf->pwdir[len] == '/')
>>>>      pwbuf->pwdir[len] = '\0';
>>>>
>>>> ?
>>>>
>>>>        
>>> What about /home/dwalsh//////
>>> Which I believe is legal
>>>      
>>>> Also, won't this fail if the homedir is set to '/' ? This check should
>>>> probably go below the strcmp(pwbuf->pw_dir, "/") that is currently below
>>>> it.
>>>>        
>>> Yes good point.
>>>
>>>      
>> I thought it worked fine with '/'. Since the loop condition is len>  0
>> (where len is the index of the last character, not the length of the string,
>> which is a bit confusing and should probably be changed), it would never
>> reset the first character. And don't you want this before the
>> strcmp(pwbuf->pw_dir, "/") in case the path is ///// (which would of course
>> be silly, but I believe is legal anyway)?
>>
>> Chad
>>
>>    
>
> Yep, got it.
>
> Acked-By: Joshua Brindle <method@manicmethod.com>

Sorry, premature ack.

I'm not seeing the expected results here. I added a user with a homedir 
of /home/method///// and the file_contexts.homedirs still has:

/home/method//////.+ staff_u:object_r:user_home_t:s0


Also, you should probably use '\0' rather than 0 when truncating the 
path. And does this get rid of the need of:

                 semanage_rtrim(path, '/');

a few lines below your patch?



[-- Attachment #2: Type: text/html, Size: 3845 bytes --]