linux-kernel - Re: [REPOST PATCH v3 2/2] vfs: parse: deal with zero length string value

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221017185523.22f43b5d7f9fee1e1e3d872f@linux-foundation.org>
Date:   Mon, 17 Oct 2022 18:55:23 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Ian Kent <raven@...maw.net>
Cc:     Al Viro <viro@...IV.linux.org.uk>,
        Siddhesh Poyarekar <siddhesh@...plt.org>,
        David Howells <dhowells@...hat.com>,
        Miklos Szeredi <miklos@...redi.hu>,
        Carlos Maiolino <cmaiolino@...hat.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [REPOST PATCH v3 2/2] vfs: parse: deal with zero length string
 value

On Tue, 20 Sep 2022 15:26:29 +0800 Ian Kent <raven@...maw.net> wrote:

> Parsing an fs string that has zero length should result in the parameter
> being set to NULL so that downstream processing handles it correctly.
> For example, the proc mount table processing should print "(none)" in
> this case to preserve mount record field count, but if the value points
> to the NULL string this doesn't happen.
> 
> ...
>
> --- a/fs/fs_parser.c
> +++ b/fs/fs_parser.c
> @@ -197,6 +197,8 @@ int fs_param_is_bool(struct p_log *log, const struct fs_parameter_spec *p,
>  		     struct fs_parameter *param, struct fs_parse_result *result)
>  {
>  	int b;
> +	if (param->type == fs_value_is_empty)
> +		return 0;
>  	if (param->type != fs_value_is_string)
>  		return fs_param_bad_value(log, param);
>  	if (!*param->string && (p->flags & fs_param_can_be_empty))
> @@ -213,6 +215,8 @@ int fs_param_is_u32(struct p_log *log, const struct fs_parameter_spec *p,
>  		    struct fs_parameter *param, struct fs_parse_result *result)
>  {
>  	int base = (unsigned long)p->data;
> +	if (param->type == fs_value_is_empty)
> +		return 0;
>  	if (param->type != fs_value_is_string)
>  		return fs_param_bad_value(log, param);
>  	if (!*param->string && (p->flags & fs_param_can_be_empty))
>
> [etcetera]

This feels wrong.  Having to check for fs_value_is_empty in so many
places makes me think "we just shouldn't have got this far".  Am I
right for once?