On Tue, Jan 22, 2013 at 7:30 PM, Mike Frysinger <vapier@xxxxxxxxxx> wrote:
> On Tuesday 22 January 2013 19:24:54 Kees Cook wrote:
>
> just nits ...
>
>> +int kmod_file_get_direct(const struct kmod_file *file)
>> +{
>> + return file->direct;
>> +}
>> +
>> +int kmod_file_get_fd(const struct kmod_file *file)
>> +{
>> + return file->fd;
>> +}
>
> these are only in the private header, so they could be static inlines since
> you don't have to worry about the ABI layout of the structure
This could be said of all the functions in the private header, right?
Currently the others aren't defined static. It looks like none of
these end up getting exported, though.
>> --- a/libkmod/libkmod-module.c
>> +++ b/libkmod/libkmod-module.c
>>
>> +#ifndef __NR_finit_module
>> +# if defined(__x86_64__)
>> +# define __NR_finit_module 313
>> +# elif defined(__i386__)
>> +# define __NR_finit_module 350
>> +# elif defined(__arm__)
>> +# define __NR_finit_module 379
>> +# endif
>> +#endif
>> +
>> +#ifdef __NR_finit_module
>> +static inline int finit_module(int fd, const char *uargs, int flags)
>> +{
>> + return syscall(__NR_finit_module, fd, uargs, flags);
>> +}
>> +#else
>> +static inline int finit_module(int fd, const char *uargs, int flags)
>> +{
>> + errno = ENOSYS;
>> + return -1;
>> +}
>> +#endif
>
> in the "ifndef __NR_finit_module" logic, you could add an:
> # else
> # define __NR_finit_module -1
>
> then you would only have one finit_module() implementation to content with
Good idea.
> you should add a configure test for the finit_module function. when (if?) glibc
> starts including it, you'll want to avoid the fallback definition.
Yeah, good idea.
>> --- a/testsuite/init_module.c
>> +++ b/testsuite/init_module.c
>>
>> +TS_EXPORT int finit_module(const int fd, const char *args, const int
>> flags);
>> +
>> +int finit_module(const int fd, const char *args, const int flags)
>
> is the double decl existing style ? i don't think it's needed:
It is -- I modeled it after the call for init_module. Without it "make
check" won't build.
> TS_EXPORT int finit_module(const int fd, const char *args, const int flags)
> {
> ...
> }
>
>> + mem = mmap(0, len, PROT_READ, MAP_PRIVATE, fd, 0);
>
> first arg should be NULL
> -mike
Thanks, I'll send a v2.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
