On 7/12/22 8:09 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Having not drank any (or maybe too much) coffee this morning, I typed:
>
> $ mkfs.xfs -d agcount=3 -d nrext64=0
> Segmentation fault
>
> I traced this down to getsubopt walking off the end of the dopts.subopts
> array. The manpage says you're supposed to terminate the suboptions
(the getsubopt(3) manpage for those following along at home)
> string array with a NULL entry, but the structure definition uses
> MAX_SUBOPTS/D_MAX_OPTS directly, which means there is no terminator.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> mkfs/xfs_mkfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 61ac1a4a..9a58ff8b 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -141,7 +141,7 @@ enum {
> };
>
> /* Just define the max options array size manually right now */
> -#define MAX_SUBOPTS D_MAX_OPTS
> +#define MAX_SUBOPTS (D_MAX_OPTS + 1)
Hah, I had not noticed this before. So this relies on there being more
suboptions for -d than anything else, I guess. What could go wrong?
OK, so this fixes it because opt_params is a global, and it contains
subopt_params[MAX_SUBOPTS];, so the last array entry will be null
(by virtue of globals being zeroed) and that's all perfectly clear :D
Well, it fixes it for now. I'd like to add i.e.
@@ -251,6 +251,7 @@ static struct opt_params bopts = {
.ini_section = "block",
.subopts = {
[B_SIZE] = "size",
+ [B_MAX_OPTS] = NULL,
},
etc to each suboption array to be explicit about it, sound ok? I can do
that on commit if it seems ok.
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
Thanks,
-Eric
>
> #define SUBOPT_NEEDS_VAL (-1LL)
> #define MAX_CONFLICTS 8
>
