Bug 9456

Summary: mkusers script bash errors
Product: buildroot Reporter: George Y. <georgebrmz>
Component: OtherAssignee: Yann E. MORIN <yann.morin.1998>
Status: RESOLVED FIXED    
Severity: normal CC: buildroot, yann.morin.1998
Priority: P5    
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: Linux   
Host: Target:
Build:
Attachments: Buildroot config
User table for paul buildroot config
Debug mkusers
Debug mkusers - paranoid
Output from mkusers script with debug paranoid patch.
Rename variable, paranoid debug
Rename variable, paranoid debug output file

Description George Y. 2016-11-28 22:49:48 UTC
This happens with the latest git as of 2016/11/28:

/buildroot/support/scripts/mkusers: line 417: [: -ge: unary operator expected
/buildroot/support/scripts/mkusers: line 426: [: -eq: unary operator expected
Comment 1 Peter Korsgaard 2016-11-29 07:27:18 UTC
(In reply to George Y. from comment #0)

What is your .config? Have you added any custom packages? It sounds like wrong values are passed to mkusers
Comment 2 George Y. 2016-11-29 12:13:53 UTC
Created attachment 6811 [details]
Buildroot config

The config is attached. Nothing specifically different.

By the way, the build process does not stop. It continues and ignores those errors, so if it happens for you as well, you'll only see it from the messages, but the building will continue and will give an impression of success.
Comment 3 Arnout Vandecappelle 2016-11-29 21:46:16 UTC
Paul__ on IRC reported the same issue. I spent some time trying to debug it, but I wasn't able to reproduce with the same users settings.

The only thing that changed in that area is pseudo, as far as I can see. However, on 2016/11/28, pseudo was already reverted. That said, the .config file that is attached to this bug says it is from 2016.11-rc2 so I'm not entirely sure if the revert is already included. Please try again with 2016.11-rc3 - if it does work there, then we have identified the culprit.
Comment 4 George Y. 2016-11-30 22:54:58 UTC
Please disregard the version number in the config file, I'm using the same config for a while without letting buildroot upgrade it, and I'm always doing a clean git clone for the master branch. I'm also on OpenSuSE distribution, if that matters.
Comment 5 Thomas Petazzoni 2017-02-13 20:44:28 UTC
George, do you still see the issue with the latest Buildroot 2017.02-rc1 ?
Comment 6 Thomas Petazzoni 2017-02-26 20:21:38 UTC
No feedback from George since end of November, so I'll close the bug as NEEDINFO. George, if you still see the problem with the latest Buildroot version, do not hesitate to reopen this bug with more details.

Thanks!
Comment 7 George Y. 2017-02-26 22:51:47 UTC
We are not making buildroot builds too often. I'll attempt a build soon, and if it still happens, I'll reopen the bug. Thanks!
Comment 8 Paul Stewart 2017-02-28 19:18:04 UTC
(In reply to Thomas Petazzoni from comment #5)

Hello is Paul__ from the irc channel.

The issue is not resolved for me with 2017.02-rc3.  It is still reporting the errors.  Because the problem occurs with the first line, my workaround is to add a dummy user before the real users.  My user table looks like this:

dummy -1 users -1 * - - - Dummy user for mkusers script bug
paul 1000 users 100 =mypassword /home/paul /bin/sh ssh-user
git 1001 users 100 =gitpassword /home/git /usr/bin/git-shell ssh-user

If you remove the first line, it will not create the paul user.  The passwd file will include the standard users followed by my git user and sshd (because the openssh package is included in my buildroot .config).

This is what it looks like if you omit the dummy line above:

root:x:0:0:root:/root:/bin/sh
daemon:x:1:1:daemon:/usr/sbin:/bin/false
bin:x:2:2:bin:/bin:/bin/false
sys:x:3:3:sys:/dev:/bin/false
sync:x:4:100:sync:/bin:/bin/sync
mail:x:8:8:mail:/var/spool/mail:/bin/false
www-data:x:33:33:www-data:/var/www:/bin/false
operator:x:37:37:Operator:/var:/bin/false
nobody:x:65534:65534:nobody:/home:/bin/false
git:x:1001:100::/home/git:/usr/bin/git-shell
sshd:x:1000:1000:SSH drop priv user:/:/bin/false
Comment 9 Yann E. MORIN 2017-02-28 21:04:05 UTC
(In reply to paulstewartis@gmail.com from comment #8)

OK, let's see what I can do with that users table. Thanks for the use-case.
Comment 10 Yann E. MORIN 2017-02-28 21:15:37 UTC
(In reply to paulstewartis@gmail.com from comment #8)

Still no luck in trying to reproduce the issue with your users table
(minus the dummy user) and the following defconfig:

BR2_arm=y
BR2_cortex_a7=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
BR2_ROOTFS_USERS_TABLES="/home/ymorin/users.list"
# BR2_PACKAGE_BUSYBOX is not set

Here is the /etc/passwd the gets me:

root:x:0:0:root:/root:/bin/sh
daemon:x:1:1:daemon:/usr/sbin:/bin/false
bin:x:2:2:bin:/bin:/bin/false
sys:x:3:3:sys:/dev:/bin/false
sync:x:4:100:sync:/bin:/bin/sync
mail:x:8:8:mail:/var/spool/mail:/bin/false
www-data:x:33:33:www-data:/var/www:/bin/false
operator:x:37:37:Operator:/var:/bin/false
nobody:x:65534:65534:nobody:/home:/bin/false
paul:x:1000:100::/home/paul:/bin/sh
git:x:1001:100::/home/git:/usr/bin/git-shell

So as you can see, both the paul and git users are there.

What is your bash version:

/usr/bin/env bash --version
Comment 11 Paul Stewart 2017-02-28 21:57:59 UTC
(In reply to Yann E. MORIN from comment #10)

paul@linux:~> /usr/bin/env bash --version
GNU bash, version 4.3.42(1)-release (x86_64-suse-linux-gnu)
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
paul@linux:~>

I can post my .config if you want too.
Comment 12 Yann E. MORIN 2017-02-28 22:10:55 UTC
(In reply to paulstewartis@gmail.com from comment #11)

OK, bash is not too old ( I have 4.3.46).

Wild guess: your users table file does not have CR-LF endings, does it?

I have absolutely no clue what's going on... :-/
Comment 13 Paul Stewart 2017-02-28 23:23:34 UTC
(In reply to Yann E. MORIN from comment #12)

"Wild guess: your users table file does not have CR-LF endings, does it?"

LF only.  I tried both by the way.  No difference either way.
Comment 14 Arnout Vandecappelle 2017-03-01 07:46:06 UTC
Perhaps there is some mojibake in the users file? Can you attach it as binary?

The bash errors make no sense at all, because exactly the same read commands and LINES array is used a bit higher up in the mkusers script. Except of course if one of the spaces is not a real space. Since mkusers is executed with LANG=C, it will most likely not split on any of the > U+007F whitespace, and you end up with e.g. uid = 1000\xc2\xa0users.
Comment 15 Paul Stewart 2017-03-01 16:19:14 UTC
Created attachment 6906 [details]
User table for paul buildroot config

Here is the actual file. The only difference from what I pasted before is the passwords.  I have tried different, simple, short passwords and it did not make a difference.  I will change these passwords, of course, now that they are posted on the internet. :-)
Comment 16 Paul Stewart 2017-03-01 16:38:45 UTC
(In reply to Arnout Vandecappelle from comment #14)

Looking at the Hexdump, it looks like 0x20 for the spaces:

00000000  64 75 6d 6d 79 20 2d 31  20 75 73 65 72 73 20 2d  |dummy -1 users -|
00000010  31 20 2a 20 2d 20 2d 20  2d 20 44 75 6d 6d 79 20  |1 * - - - Dummy |
<snip>
00000110  72 2f 62 69 6e 2f 67 69  74 2d 73 68 65 6c 6c 20  |r/bin/git-shell |
00000120  73 73 68 2d 75 73 65 72  0a                       |ssh-user.|
00000129
Comment 17 Yann E. MORIN 2017-03-01 17:30:07 UTC
(In reply to Paul Stewart from comment #15)

So I downloaded that file and uses it:

  - as-is, and it creates the three users: dummy, paul and git;
  - without the dummy user, and it creates paul and git.

I'll try to instrument the script and attach a patch so that you can report the output. Give me a moment...

Regards,
Yann E. MORIN.
Comment 18 Yann E. MORIN 2017-03-01 17:46:04 UTC
Created attachment 6911 [details]
Debug mkusers

Paul,

Could you test with the attched patch and report the result, please?

Here is what I got:

line='paul 1000 users 100 =21248f7aac088184a7c9dd3a3cd8bd1bafecc5718174e9025bab7053756a013a /home/paul /bin/sh ssh-user'
  username='paul'
  uid     ='1000'
  group   ='users'
  gid     ='100'
  paswd   ='=21248f7aac088184a7c9dd3a3cd8bd1bafecc5718174e9025bab7053756a013a'
  home    ='/home/paul'
  shell   ='/bin/sh'
  groups  ='ssh-user'
  comment =''
line='git 1001 users 100 =de5d4f76680ec42ff304682eebeaec1c4756e3d5de3e4ce6db747cb223d6037c /home/git /usr/bin/git-shell ssh-user'
  username='git'
  uid     ='1001'
  group   ='users'
  gid     ='100'
  paswd   ='=de5d4f76680ec42ff304682eebeaec1c4756e3d5de3e4ce6db747cb223d6037c'
  home    ='/home/git'
  shell   ='/usr/bin/git-shell'
  groups  ='ssh-user'
  comment =''
Comment 19 Paul Stewart 2017-03-01 22:41:15 UTC
(In reply to Yann E. MORIN from comment #18)

line='paul 1000 users 100 =21248f7aac088184a7c9dd3a3cd8bd1bafecc5718174e9025bab7053756a013a /home/paul /bin/sh ssh-user'
  username='paul'
  uid     ='1000'
  group   ='users'
  gid     ='100'
  paswd   ='=21248f7aac088184a7c9dd3a3cd8bd1bafecc5718174e9025bab7053756a013a'
  home    ='/home/paul'
  shell   ='/bin/sh'
  groups  ='ssh-user'
  comment =''
line='git 1001 users 100 =de5d4f76680ec42ff304682eebeaec1c4756e3d5de3e4ce6db747cb223d6037c /home/git /usr/bin/git-shell ssh-user'
  username='git'
  uid     ='1001'
  group   ='users'
  gid     ='100'
  paswd   ='=de5d4f76680ec42ff304682eebeaec1c4756e3d5de3e4ce6db747cb223d6037c'
  home    ='/home/git'
  shell   ='/usr/bin/git-shell'
  groups  ='ssh-user'
  comment =''
line='sshd -1 sshd -1 * - - - SSH drop priv user'
  username='sshd'
  uid     ='-1'
  group   ='sshd'
  gid     ='-1'
  paswd   ='*'
  home    ='-'
  shell   ='-'
  groups  ='-'
  comment ='SSH drop priv user'

This is immediately followed by:
/home/paul/src/buildroot/support/scripts/mkusers: line 407: [: -eq: unary operator expected
/home/paul/src/buildroot/support/scripts/mkusers: line 432: [: -ge: unary operator expected
/home/paul/src/buildroot/support/scripts/mkusers: line 441: [: -eq: unary operator expected

And the paul user is not created.
Comment 20 Paul Stewart 2017-03-02 00:09:44 UTC
I created a default rpi 2 config and set it to use my user table.  And I get the same issue.  So it isn't my .config file neither.
Comment 21 Yann E. MORIN 2017-03-02 17:28:24 UTC
Created attachment 6916 [details]
Debug mkusers - paranoid

(In reply to Paul Stewart from comment #19)

Damn, that's uterly weird...

OK, so let's be a little bit more paranoiac. Can you test with the attached patch, please?

The expected output is described in that patch, too.

Regards,
Yann E. MORIN.
Comment 22 Paul Stewart 2017-03-02 23:17:37 UTC
Created attachment 6921 [details]
Output from mkusers script with debug paranoid patch.

Attached output of paranoid patched mkusers.
Comment 23 Arnout Vandecappelle 2017-03-03 15:26:11 UTC
D'oh!

From the bash man page:

LINES  Used by the select compound command to determine the column length for 
       printing selection lists.  Automatically  set  if  the  checkwinsize
       option  is enabled or in an interactive shell upon receipt of a SIGWINCH.

Your bash is receiving a SIGWINCH from your terminal and updates the LINES variable when it generates some output...

Yann, please rename LINES to a non-conflicting variable :-)
Comment 24 Yann E. MORIN 2017-03-03 17:31:53 UTC
(In reply to Arnout Vandecappelle from comment #23)

I'll do the change, for sanity sake.

However:
  - the checkwinsize option is disabled by default in a non interactive shell,
  - this is not interactive shell, so SIGWINCH should be ignored.

I'll paste a patch in a moment...

Regards,
Yann E. MORIN.
Comment 25 Yann E. MORIN 2017-03-03 17:37:10 UTC
Created attachment 6926 [details]
Rename variable, paranoid debug

Paul,

Could you please test this patch and report the output, please?

For the record, the output here contains:

    checkwinsize    off

Regards,
Yann E. MORIN.
Comment 26 Paul Stewart 2017-03-03 21:50:13 UTC
(In reply to Yann E. MORIN from comment #25)

That produced the correct output.  The passwd, group and shadow files were all created properly.  Yay!  Will post the output file momentarily.
Comment 27 Paul Stewart 2017-03-03 21:57:18 UTC
Created attachment 6931 [details]
Rename variable, paranoid debug output file

Here is the (successful) output from the renamed variable patch.
Comment 28 Yann E. MORIN 2017-03-03 22:10:58 UTC
(In reply to Paul Stewart from comment #27)

Thanks a lot for all the testing you did! :-)

This is really weird that your shell does have checkwinsize on. It
should be off by default in a non-interactive shell, and shell script
are running in non-interactive shells...

Anyway... I just sent the patch:

    http://lists.busybox.net/pipermail/buildroot/2017-March/185755.html

Thanks again!

Regards,
Yann E. MORIN.
Comment 29 Paul Stewart 2017-03-03 23:06:11 UTC
(In reply to Yann E. MORIN from comment #28)

You're welcome.  Thank you for the fix!  Will this go into a 2017.02.1 bugfix or will it wait for the 2017.05 release?
Comment 30 Yann E. MORIN 2017-03-04 10:05:06 UTC
(In reply to Paul Stewart from comment #29)

Since we decided to have 2017.02 be our first long-term maintainance branch,
I believe this will go in, yes.
Comment 31 Thomas Petazzoni 2017-03-04 10:58:04 UTC
Fixed by https://git.buildroot.org/buildroot/commit/?id=00d34e8a6f378653a384c66d68f9a65e13b8034f. Thanks Paul, Arnout and Yann for the investigation!