| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 28 Feb 2015 14:28:01 -0300
From: Lucas De Marchi <lucas.de.marchi@...il.com>
To: Harish Jenny Kandiga Nagaraj <harish_kandiga@...tor.com>
Cc: Rusty Russell <rusty@...tcorp.com.au>,
linux-modules <linux-modules@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>,
greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN
On Thu, Feb 19, 2015 at 12:35 PM, Harish Jenny Kandiga Nagaraj
<harish_kandiga@...tor.com> wrote:
>
> On Thursday 19 February 2015 07:32 PM, Harish Jenny Kandiga Nagaraj wrote:
>> On Thursday 19 February 2015 06:13 PM, Lucas De Marchi wrote:
>>> On Thu, Feb 19, 2015 at 10:32 AM, Harish Jenny Kandiga Nagaraj
>> diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
>> index 417f232..f6ffd3e 100644
>> --- a/libkmod/libkmod-internal.h
>> +++ b/libkmod/libkmod-internal.h
>> @@ -46,6 +46,13 @@ static _always_inline_ _printf_format_(2, 3) void
>> # endif
>> #endif
>>
>> +/* Flags to kmod_builtin_status() */
>> +enum kmod_builtin_status {
>> + KMOD_BUILTIN_UNKNOWN = 0,
>> + KMOD_BUILTIN_NO = 1,
>> + KMOD_BUILTIN_YES = 2
>> +};
>> +
>> void kmod_log(const struct kmod_ctx *ctx,
>> int priority, const char *file, int line, const char *fn,
>> const char *format, ...) __attribute__((format(printf, 6, 7))) __attribute__((nonnull(1, 3, 5)));
>> @@ -92,6 +99,7 @@ int kmod_lookup_alias_from_symbols_file(struct kmod_ctx *ctx, const char *name,
>> int kmod_lookup_alias_from_aliases_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
>> int kmod_lookup_alias_from_moddep_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
>> int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
>> +int kmod_just_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name)__attribute__((nonnull(1, 2)));
>> int kmod_lookup_alias_from_commands(struct kmod_ctx *ctx, const char *name, struct kmod_list **list) __attribute__((nonnull(1, 2, 3)));
>> void kmod_set_modules_visited(struct kmod_ctx *ctx, bool visited) __attribute__((nonnull((1))));
>> void kmod_set_modules_required(struct kmod_ctx *ctx, bool required) __attribute__((nonnull((1))));
>> @@ -143,9 +151,9 @@ int kmod_module_parse_depline(struct kmod_module *mod, char *line) __attribute__
>> void kmod_module_set_install_commands(struct kmod_module *mod, const char *cmd) __attribute__((nonnull(1)));
>> void kmod_module_set_remove_commands(struct kmod_module *mod, const char *cmd) __attribute__((nonnull(1)));
>> void kmod_module_set_visited(struct kmod_module *mod, bool visited) __attribute__((nonnull(1)));
>> -void kmod_module_set_builtin(struct kmod_module *mod, bool builtin) __attribute__((nonnull((1))));
>> +void kmod_module_set_builtin(struct kmod_module *mod, enum kmod_builtin_status builtin) __attribute__((nonnull((1))));
>> void kmod_module_set_required(struct kmod_module *mod, bool required) __attribute__((nonnull(1)));
>> -
>> +bool kmod_module_is_builtin(const struct kmod_module *mod);
>>
>> /* libkmod-file.c */
>> struct kmod_file *kmod_file_open(const struct kmod_ctx *ctx, const char *filename) _must_check_ __attribute__((nonnull(1,2)));
>> diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
>> index 19bb2ed..6b2f2f1 100644
>> --- a/libkmod/libkmod-module.c
>> +++ b/libkmod/libkmod-module.c
>> @@ -98,7 +98,7 @@ struct kmod_module {
>> * is set. There's nothing much useful one can do with such a
>> * "module", except knowing it's builtin.
>> */
>> - bool builtin : 1;
>> + enum kmod_builtin_status builtin;
>> };
>>
>> static inline const char *path_join(const char *path, size_t prefixlen,
>> @@ -210,7 +210,7 @@ void kmod_module_set_visited(struct kmod_module *mod, bool visited)
>> mod->visited = visited;
>> }
>>
>> -void kmod_module_set_builtin(struct kmod_module *mod, bool builtin)
>> +void kmod_module_set_builtin(struct kmod_module *mod, enum kmod_builtin_status builtin)
>> {
>> mod->builtin = builtin;
>> }
>> @@ -220,6 +220,15 @@ void kmod_module_set_required(struct kmod_module *mod, bool required)
>> mod->required = required;
>> }
>>
>> +bool kmod_module_is_builtin(const struct kmod_module *mod)
>> +{
>> + int builtin = mod->builtin;
>> +
>> + if (builtin == KMOD_BUILTIN_UNKNOWN)
>> + builtin = kmod_just_lookup_alias_from_builtin_file(mod->ctx, mod->name);
>> +
>> + return mod->builtin == KMOD_BUILTIN_YES;
>> +}
>> /*
>> * Memory layout with alias:
>> *
>> @@ -924,7 +933,7 @@ KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx,
>> module_is_blacklisted(mod))
>> continue;
>>
>> - if ((filter_type & KMOD_FILTER_BUILTIN) && mod->builtin)
>> + if ((filter_type & KMOD_FILTER_BUILTIN) && kmod_module_is_builtin(mod))
>> continue;
>>
>> node = kmod_list_append(*output, mod);
>> @@ -1713,7 +1722,7 @@ KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod)
>> if (mod == NULL)
>> return -ENOENT;
>>
>> - if (mod->builtin)
>> + if (kmod_module_is_builtin(mod))
>> return KMOD_MODULE_BUILTIN;
>>
>> pathlen = snprintf(path, sizeof(path),
>> diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
>> index 1a5a66b..d79bb12 100644
>> --- a/libkmod/libkmod.c
>> +++ b/libkmod/libkmod.c
>> @@ -525,7 +525,7 @@ int kmod_lookup_alias_from_builtin_file(struct kmod_ctx *ctx, const char *name,
>> goto finish;
>> }
>>
>> - kmod_module_set_builtin(mod, true);
>> + kmod_module_set_builtin(mod, KMOD_BUILTIN_YES);
>> *list = kmod_list_append(*list, mod);
>> if (*list == NULL)
>> err = -ENOMEM;
>> @@ -536,6 +536,39 @@ finish:
>> return err;
>> }
>>
>> +int kmod_just_lookup_alias_from_builtin_file(struct kmod_ctx *ctx,
>> + const char *name) {
>> + char *line = NULL;
>> +
>> + if (ctx->indexes[KMOD_INDEX_MODULES_BUILTIN]) {
>> + DBG(ctx, "use mmaped index '%s' modname=%s\n",
>> + index_files[KMOD_INDEX_MODULES_BUILTIN].fn, name);
>> + line = index_mm_search(ctx->indexes[KMOD_INDEX_MODULES_BUILTIN], name);
>> + } else {
>> + struct index_file *idx;
>> + char fn[PATH_MAX];
>> +
>> + snprintf(fn, sizeof(fn), "%s/%s.bin", ctx->dirname,
>> + index_files[KMOD_INDEX_MODULES_BUILTIN].fn);
>> + DBG(ctx, "file=%s modname=%s\n", fn, name);
>> +
>> + idx = index_file_open(fn);
>> + if (idx == NULL) {
>> + DBG(ctx, "could not open builtin file '%s'\n", fn);
>> + goto finish;
>> + }
>> +
>> + line = index_search(idx, name);
>> + index_file_close(idx);
>> + }
>> +
>> + if (line != NULL)
>> + return KMOD_BUILTIN_YES;
>> +
>> +finish:
>> + return KMOD_BUILTIN_NO;
>> +}
>> +
>> char *kmod_search_moddep(struct kmod_ctx *ctx, const char *name)
>> {
>> struct index_file *idx;
>>
>>
>>
>
>
> I guess there are some mistakes
>
> 1) return mod->builtin == KMOD_BUILTIN_YES; should be made to
> return builtin == KMOD_BUILTIN_YES;
> 2) S_ISDIR check needs to be removed.
>
> Lucas,
> In any case , Can this patch can be taken in sequence?
> My first original patch of removing directory check can be taken first. Then the necessary changes required. (you might add as well in your free time as suggested by you)
I fixed some mistakes like you pointed out, added minor changes and
applied to the master branch:
https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4
I also added some tests to the testsuite on top. Could you please take
a look if everything is right for your use case in master branch?
thanks
--
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists