On Wed, Jul 13, 2022 at 08:39:24PM -0500, Eric Sandeen wrote:
> 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
<nod>
> 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.
Oh, that /is/ a good idea, in case B_MAX_OPTS > D_MAX_OPTS ever happens.
--D
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
>
> Thanks,
> -Eric
>
> >
> > #define SUBOPT_NEEDS_VAL (-1LL)
> > #define MAX_CONFLICTS 8
> >
