On 06/01/2015 10:47 AM, Tyler Hicks wrote:
> On 2015-06-01 10:18:34, John Johansen wrote:
>> On 06/01/2015 06:46 AM, Tyler Hicks wrote:
>>> On 2015-05-31 18:00:25, Christian Boltz wrote:
>>>> Hello,
>>>>
>>>> Am Freitag, 29. Mai 2015 schrieb Tyler Hicks:
>>>>> On 2015-05-30 00:00:25, Christian Boltz wrote:
>>>>>> Am Freitag, 29. Mai 2015 schrieb Tyler Hicks:
>>>>>>> On 2015-05-29 01:39:15, John Johansen wrote:
>>>>>>>> +int aa_query_file(uint32_t mask, const char *label, const char
>>>>>>>> *path, + int *allowed, int *audited)
>>>>>>>
>>>>>>> I prefer that we require 'size_t label_len' and 'size_t path_len'
>>>>>>> parameters. The caller may already have the string lengths stored
>>>>>>> in
>>>>>>> variables, eliminating unnecessary calls to strlen(). Also, it
>>>>>>> allows
>>>>>>> for non-nul-terminated strings to be used.
>>>>>>
>>>>>> You mean you want to call the function with path "foo\0" and
>>>>>> path_len
>>>>>> 12345?
>>>>>>
>>>>>> Personally, I prefer an unnecessary strlen() call over an option to
>>>>>> allow someone to hand in invalid data (and, caused by that, possibly
>>>>>> doing funny[tm] things) ;-)
>>>>>
>>>>> You may not be aware that strlen() requires the string to be
>>>>> nul-terminated. If they wanted to shoot themselves in the foot or "do
>>>>> funny things" they could just pass in a non nul-terminated string to
>>>>> aa_query_file().
>>>>
>>>> I don't know too much about C, but I already heard that strings must be
>>>> \0-terminated ;-) and I'm not really surprised that strlen() needs the
>>>> \0 to find the end.
>>>>
>>>>> Also, libapparmor is in the process' address space. It makes no
>>>>> difference if we allow the caller to specify the string length or
>>>>> not...
>>>>
>>>> So basically we have to decide if we
>>>> a) reduce the number of function parameters, which makes it easier for
>>>> callers, but also means that the string _must be_ \0-terminated
>>>> b) have a parameter for the string len, which could possibly come with a
>>>> wrong value
>>>>
>>>> At least for non-C callers (we have perl/python/whatever bindings for
>>>> libapparmor), option a) sounds better to me.
>>>
>>> The string length parameters really only make sense for the C API. The
>>> bindings for higher level languages probably shouldn't require string
>>> length parameters.
>>>
>> Well in the file path case, an embedded \0 is not allowed as part of the
>> query so passing the arg is a performance optimization.
>>
>> These helper fns are just wrappers around the query api, so that apps
>> don't need to know the format of the queries.
>>
>> I'm fine with putting in the length parameter but I find it weired that
>> we would want length in the C api and not in other languages. Also since
>> currently our other language apis are generated with swig, the length
>> parameter argument will likely carry over.
>>
>> I'm fine with putting the length in as an arg, I just want to make sure
>> that is what we want to do
>
> int aa_query_file_len(uint32_t mask, const char *label, size_t label_len,
> const char *path, size_t path_len,
> int *allowed, int *audited)
> {
> ...
> }
>
> int aa_query_file_nul(uint32_t mask, const char *label, const char *path,
> int *allowed, int *audited)
> {
> return aa_query_file(mask, label, strlen(label), path,
> strlen(path), allowed, audited);
> }
>
we could, though I think
aa_query_file_path()
and
aa_query_file_path_len()
might be better names. I've started thinking we should be very explicit that
this is a path query, especially since we have been thinking about a file/fd
query.
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
