[PATCH 1/1] makedevs: set path size to match linux

Kang-Che Sung explorer09 at gmail.com
Tue Jun 27 13:20:22 UTC 2017


On Tue, Jun 27, 2017 at 3:53 PM, walter harms <wharms at bfs.de> wrote:
>
> Am 27.06.2017 05:31, schrieb Baruch Siach:
>>
>> On Mon, Jun 26, 2017 at 08:45:42PM -0500, Matthew Weber wrote:
>>> On Mon, Jun 26, 2017 at 7:36 PM, Emmanuel Deloget <logout at free.fr> wrote:
>>>> On Mon, Jun 26, 2017 at 11:23 PM, Matthew Weber
>>>> <matthew.weber at rockwellcollins.com> wrote:
>>>>> On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach <baruch at tkos.co.il> wrote:
>>>>>> On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
>>>>>>> From: Jared Bents <jared.bents at rockwellcollins.com>
>>>>>>>
>>>>>>> Update to increase the pathname limit to the
>>>>>>> linux limit of 4096 characters.
>>>>>>>
>>>>>>> Similar patch:
>>>>>>> https://patchwork.openembedded.org/patch/131475/
>>>>>>>
>>>>>>> Signed-off-by: Jared Bents <jared.bents at rockwellcollins.com>
>>>>>>> Signed-off-by: Matt Weber <matthew.weber at rockwellcollins.com>
>>>>>>> ---
>>>>>>>  miscutils/makedevs.c | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/miscutils/makedevs.c b/miscutils/makedevs.c
>>>>>>> index 9e7ca34..0049edb 100644
>>>>>>> --- a/miscutils/makedevs.c
>>>>>>> +++ b/miscutils/makedevs.c
>>>>>>> @@ -208,7 +208,7 @@ int makedevs_main(int argc UNUSED_PARAM, char
>>>>>>> **argv)
>>>>>>>               unsigned count = 0;
>>>>>>>               unsigned increment = 0;
>>>>>>>               unsigned start = 0;
>>>>>>> -             char name[41];
>>>>>>> +             char name[4096];
>>>>>>
>>>>>> Why not use PATH_MAX here?
>>>>>
>>>>> Agree, that would be cleaner.  Will submit v2 after some testing.
>>>>>
>>>>> That still leaves a hardcoded value in the sscanf  of 4095........
>>>>> should I add a comment to the affect we're assuming PATH_MAX is at
>>>>> least 4096?  Maybe a check is also needed?
>>>>
>>>>
>>>> Alternatively you may use the m modifier, when implemented, to auto-allocate
>>>> the name pointer. This way you don't have to hardcode anything in the
>>>> sscanf(), you let the library for the job for you. Later, you can check the
>>>> string length.
>>>>
>>>> Such a change would induce an allocation, a free but will also reduce stack
>>>> usage.
>>>>
>>>
>>> If we want to keep it all static, another option would be to stringify
>>> that define.
>>> (Courtesy Jared for this idea)
>>>
>>> + #define STRINGIFY(x) STRINGIFY2(x)
>>> + #define STRINGIFY2(x) #x
>>> .....
>>> - if ((2 > sscanf(line, "%40s %c %o %40s %40s %u %u %u %u %u",
>>> + if ((2 > sscanf(line, "%" STRINGIFY(PATH_MAX) "s %c %o %40s %40s %u
>>> %u %u %u %u",
>>
>> You need 'STRINGIFY(PATH_MAX-1)'. I'm not sure this does what you mean.
>>
>
> May i ask "what is the problem (beyond truncation) what we want to solve" ?
> It there a need for "unlimited" length ?
>
> Independed of that it would be possible the use the %m modifier for scanf.
> (and break older verions of glibc).
> see: https://stackoverflow.com/questions/38685724/difference-between-ms-and-s-scanf
>

Hello. Can I suggest another way?
How about modifying the `line` buffer so it can be used directly as the name,
and avoid the malloc problem or STRINGIFY(PATH_MAX) altogether.

My suggestion (warning, this code is not tested):

--- a/miscutils/makedevs.c
+++ b/miscutils/makedevs.c
@@ -209,23 +209,27 @@ int makedevs_main(int argc UNUSED_PARAM, char **argv)
  unsigned increment = 0;
  unsigned start = 0;
  char name[41];
+ int name_len;
  char user[41];
  char group[41];
- char *full_name = name;
+ char *full_name;
  uid_t uid;
  gid_t gid;

  linenum = parser->lineno;

- if ((2 > sscanf(line, "%40s %c %o %40s %40s %u %u %u %u %u",
- name, &type, &mode, user, group,
+ if ((1 > sscanf(line, "%*s%n %c %o %40s %40s %u %u %u %u %u",
+ &name_len, &type, &mode, user, group,
  &major, &minor, &start, &increment, &count))
+ || (PATH_MAX > name_len + 1)
  || ((unsigned)(major | minor | start | count | increment) > 255)
  ) {
  bb_error_msg("invalid line %d: '%s'", linenum, line);
  ret = EXIT_FAILURE;
  continue;
  }
+ line[name_len] = '\0';
+ full_name = line;

  gid = (*group) ? get_ug_id(group, xgroup2gid) : getgid();
  uid = (*user) ? get_ug_id(user, xuname2uid) : getuid();


More information about the busybox mailing list