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

[Matthew Weber]

All,

On Tue, Jun 27, 2017 at 8:36 AM, Jared Bents
<jared.be...@rockwellcollins.com> wrote:
> On Mon, Jun 26, 2017 at 10:31 PM, Baruch Siach <bar...@tkos.co.il> wrote:
>> Hi Matthew,
>>
>> On Mon, Jun 26, 2017 at 08:45:42PM -0500, Matthew Weber wrote:
>>> On Mon, Jun 26, 2017 at 7:36 PM, Emmanuel Deloget <log...@free.fr> wrote:
>>> > On Mon, Jun 26, 2017 at 11:23 PM, Matthew Weber
>>> > <matthew.we...@rockwellcollins.com> wrote:
>>> >> On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach <bar...@tkos.co.il> wrote:
>>> >> > On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
>>> >> >> From: Jared Bents <jared.be...@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.be...@rockwellcollins.com>
>>> >> >> Signed-off-by: Matt Weber <matthew.we...@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.
>>

Superseded by: http://lists.busybox.net/pipermail/busybox/2017-July/085570.html

Matt
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

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

[Matthew Weber]

Baruch/Emmanuel,

On Mon, Jun 26, 2017 at 7:36 PM, Emmanuel Deloget <log...@free.fr> wrote:
> Hello,
>
> On Mon, Jun 26, 2017 at 11:23 PM, Matthew Weber
> <matthew.we...@rockwellcollins.com> wrote:
>>
>> Baruch,
>>
>> On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach <bar...@tkos.co.il> wrote:
>> > Hi Jared,
>> >
>> > On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
>> >> From: Jared Bents <jared.be...@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.be...@rockwellcollins.com>
>> >> Signed-off-by: Matt Weber <matthew.we...@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",


Matt
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

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

[Matthew Weber]

Baruch,

On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach  wrote:
> Hi Jared,
>
> On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
>> From: Jared Bents 
>>
>> 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 
>> Signed-off-by: Matt Weber 
>> ---
>>  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?

Matt
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/1] coreutil mkdir: ignore -z when selinux is runtime disabled

[Matthew Weber]

Denys,

On Tue, May 12, 2015 at 6:36 PM, Denys Vlasenko
vda.li...@googlemail.com wrote:
 Is this behavior compatible with standard coreutils?

http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/mkdir.c
.
case 'Z':
  if (is_smack_enabled ())
{
  /* We don't yet support -Z to restore context with SMACK.  */
  scontext = optarg;
}
  else if (is_selinux_enabled ()  0)
{
  if (optarg)
scontext = optarg;
  else
options.set_security_context = true;
}
  else if (optarg)
{
  error (0, 0,
 _(warning: ignoring --context; 
   it requires an SELinux/SMACK-enabled kernel));
}
  break;

The first arg of the error() if not 0 exits the application.  So in
this case Coreutils prints a warning and doesn't error out.


 On Mon, May 11, 2015 at 4:00 PM, Matt Weber
 matthew.we...@rockwellcollins.com wrote:
 Fixes the case of using mkdir in inittab where a system might boot
 with selinux disable during testing and still needs the folders created
 by this command for ram mounts, etc before a mount -a.  Currently it
 errors out and doesn't create the folder.

 Signed-off-by: Matthew Weber matthew.we...@rockwellcollins.com
 ---
  coreutils/mkdir.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/coreutils/mkdir.c b/coreutils/mkdir.c
 index 864edfb..9fb6e7e 100644
 --- a/coreutils/mkdir.c
 +++ b/coreutils/mkdir.c
 @@ -83,8 +83,11 @@ int mkdir_main(int argc UNUSED_PARAM, char **argv)
 flags |= FILEUTILS_VERBOSE;
  #if ENABLE_SELINUX
 if (opt  8) {
 -   selinux_or_die();
 -   setfscreatecon_or_die(scontext);
 +   if (is_selinux_enabled()) {
 +   setfscreatecon_or_die(scontext);
 +   }
 +   else
 +   bb_perror_msg(Ignored -Z for [%s],*(argv+optind));
 }

 Move argv += optind; above this code block -
 then you can use argv[0] instead of *(argv+optind),
 which is less code.

Sure


 The message should start with lowercase letter
 and use '%s', not [%s].

Sure, I hadn't though about it, but should I partially match the
coreutils output?  Maybe  -Z requires an SELinux enabled kernel,
ignored for %s


-- 
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / Security Systems and Software / Secure Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted
Third Party Intellectual Property (TPIP) content must be encrypted and
sent to matthew.we...@corp.rockwellcollins.com.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox