[systemd-devel] [PATCH] localed: validate set-x11-keymap input

[Jan Synacek]

Lennart Poettering <lennart at poettering.net> writes:
> On Tue, 04.11.14 12:05, Jan Synacek (jsynacek at redhat.com) wrote:
>
> One more addition:
>
>> +}
>> +
>> +int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char *layout_prefix)
>> +{
>> +        _cleanup_fclose_ FILE *f;
>> +        char line[LINE_MAX];
>> +        enum keymap_state state = NONE;
>> +        int r;
>> +
>> +        f = fopen("/usr/share/X11/xkb/rules/base.lst", "re");
>> +        if (!f) {
>> +                log_error("Failed to open keyboard mapping list. %m");
>> +                return -errno;
>> +        }
>> +
>> +        FOREACH_LINE(line, f, break) {
>> +                char *l, *w;
>> +
>> +                l = strstrip(line);
>> +
>> +                if (isempty(l))
>> +                        continue;
>> +
>> +                if (l[0] == '!') {
>> +                        if (startswith(l, "! model"))
>> +                                state = MODELS;
>> +                        else if (startswith(l, "! layout"))
>> +                                state = LAYOUTS;
>> +                        else if (startswith(l, "! variant"))
>> +                                state = VARIANTS;
>> +                        else if (startswith(l, "! option"))
>> +                                state = OPTIONS;
>> +                        else
>> +                                state = NONE;
>> +
>> +                        continue;
>> +                }
>> +
>> +                if (state != look_for)
>> +                        continue;
>> +
>> +                w = l + strcspn(l, WHITESPACE);
>> +
>> +                if (layout_prefix) {
>> +                        char *e;
>> +
>> +                        if (*w == 0)
>> +                                continue;
>> +
>> +                        *w = 0;
>> +                        w++;
>> +                        w += strspn(w, WHITESPACE);
>> +
>> +                        e = strchr(w, ':');
>> +                        if (!e)
>> +                                continue;
>> +
>> +                        *e = 0;
>> +
>> +                        if (!streq(w, layout_prefix))
>> +                                continue;
>> +                } else
>> +                        *w = 0;
>> +
>> +                r = strv_extend(list, l);
>> +                if (r < 0)
>> +                        return log_oom();
>
>
> I think, while we are at it, this should really be reworked to use
> GREEDY_REALLOC. See strv_split_quoted() for an example.

Could you please explain why? str_extend() uses realloc_multiply()
inside, which, to me, seems to be ok for this case.

>> +        }
>> +
>> +        if (strv_isempty(*list)) {
>> +                log_error("Couldn't find any entries."); /* TODO: improve error message */
>> +                return -ENOENT;
>> +        }
>> +
>> +        return 0;
>
>
> Lennart
>
> -- 
> Lennart Poettering, Red Hat

-- 
Jan Synacek
Software Engineer, Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20141111/e4a7dc55/attachment.sig>