On 7/13/22 8:59 PM, Darrick J. Wong wrote:
> 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.
I, uh, think that in that case, gcc will barf out with something like:
xfs_mkfs.c:311:3: error: array index in initializer exceeds array bounds
[D_MAX_OPTS] = NULL,
^
xfs_mkfs.c:311:3: error: (near initialization for ‘dopts.subopts’)
xfs_mkfs.c:311:3: warning: excess elements in array initializer [enabled by default]
xfs_mkfs.c:311:3: warning: (near initialization for ‘dopts.subopts’) [enabled by default]
cc1: warning: unrecognized command line option "-Wno-address-of-packed-member" [enabled by default]
gmake[2]: *** [xfs_mkfs.o] Error 1
gmake[1]: *** [mkfs] Error 2
make: *** [default] Error 2
(with s/dopts/bopts/ in your case)
-Eric
