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);
}
Best of both worlds?
Tyler
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
