mount.ecryptfs_private is broken on arm

[Serge Hallyn]

===============================================
SRU Justification:
1. Impact: mount.ecryptfs_private fails (with an infinite loop) on arm
2. Development fix: the infinite loop happens because an unsigned char is being compared to -1 as a condition to end the loop
3. Stable fix: same as development fix
4. Test case: run 'mount.ecryptfs_private ab' on arm
5. Regression potential: I used this updated code (on arm) for a week with no issues. If there were a regression it should have been seen (at argument parsing time) during regular use of mount.ecryptfs_private.
===============================================
char is unsigned on arm (signed on x86).

fetch_sig compares the result of fgetc to EOF (-1), which of course never succeeds, causing an infinite loop.

[Tyler Hicks (tyhicks)]

The mount.ecryptfs_private.c changes look good to me.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "debdiff with proposed fix, works for me." of this bug report has been identified as being a patch in the form of a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

[Colin Watson (cjwatson)]

The patch itself looks fine, but oneiric has released. I assume you still think this should be fixed in oneiric (it does seem to meet the SRU criteria), so could you please attach a patch for precise if necessary and also a patch targeted at oneiric-proposed? I wouldn't want to upload an SRU before it's been fixed in precise.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ecryptfs-utils - 93-0ubuntu2

---------------
ecryptfs-utils (93-0ubuntu2) precise; urgency=low

  * fix infinite loop on arm: fgetc returns an int, and -1 at end of
    options. Arm makes char unsigned. (LP: #884407)
 -- Serge Hallyn <email address hidden> Tue, 08 Nov 2011 10:47:03 -0600

[Jamie Strandboge (jdstrand)]

Thanks for the patches! As you can see, I have uploaded this to precise. I also uploaded to oneiric-proposed. Please follow https://wiki.ubuntu.com/StableReleaseUpdates to make sure this bug is noticed by the SRU team. Unsubscribing ubuntu-sponsors.

[Martin Pitt (pitti)]

Hello Serge, or anyone else affected,

Accepted ecryptfs-utils into oneiric-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Robie Basak (racb)]

Failed to reproduce on a Pandaboard running Oneiric (not -proposed):

ubuntu@panda-test:~$ mount.ecryptfs_private ab
Bad file
Error reading configuration fileubuntu@panda-test:~$ dpkg-query -W ecryptfs-utils
ecryptfs-utils 92-0ubuntu1
ubuntu@panda-test:~$ uname -a
Linux panda-test 3.0.0-1206-omap4 #13-Ubuntu SMP PREEMPT Wed Nov 23 17:50:31 UTC 2011 armv7l armv7l armv7l GNU/Linux
ubuntu@panda-test:~$

ecryptfs seems to work fine.

Some help please?

[Serge Hallyn (serge-hallyn)]

Thanks for trying, Robie!

Could you try the following program and show the result?

#include <stdio.h>

int main()
{
 char i = -1;
 if (i < 0)
  printf("char is signed\n");
 else
  printf("char is unsigned\n");
}

[Robie Basak (racb)]

Serge,

ubuntu@panda-test:~$ gcc -o test test.c
ubuntu@panda-test:~$ ./test
char is unsigned

Comparing a char to -1 is undefined, so perhaps it's being optimised away in this case?

[Robie Basak (racb)]

I tried building ecryptfs-tools from oneiric with optimisation turned off, and that doesn't cause a hang either. I've also tried the -proposed package and that also appears to work fine.

Storing the result of fgetc in a char and then comparing to EOF is clearly wrong (which is why fgetc returns an int in the first place). So although I cannot reproduce the original symptom, this change does not appear to cause a regression but does fix a bug for somebody, so I think it would be fine to go into -updates unless there is a policy that prevents this.

[Serge Hallyn (serge-hallyn)]

Quoting Robie Basak (<email address hidden>):
> Serge,
>
> ubuntu@panda-test:~$ gcc -o test test.c
> ubuntu@panda-test:~$ ./test
> char is unsigned
>
> Comparing a char to -1 is undefined, so perhaps it's being optimised
> away in this case?

No, run it on an x86 and you'll get 'char is signed'. I was hoping your
arm toolchain was somehow different from mine.

Perhaps my test case was too brief. Could you try:

mkdir /home/ubuntu/.ecryptfs
mkdir /home/ubuntu/a /home/ubuntu/b
cat > /home/ubuntu/.ecryptfs/ab.conf << EOF
/home/ubuntu/a /home/ubuntu/b ecryptfs none 0 0
EOF
ecryptfs-wrap-passphrase /home/ubuntu/ab.sig
(type 'x\n' twice)

then to try the mounting:

ecryptfs-add-passphrase
 (type 'x\n')
mount.ecryptfs_private ab

[Robie Basak (racb)]

No, I still can't get it to fail.

ubuntu@panda-test:~$ mkdir /home/ubuntu/.ecryptfs
ubuntu@panda-test:~$ mkdir /home/ubuntu/a /home/ubuntu/b
ubuntu@panda-test:~$ cat > /home/ubuntu/.ecryptfs/ab.conf << EOF
> /home/ubuntu/a /home/ubuntu/b ecryptfs none 0 0
> EOF
ubuntu@panda-test:~$ ecryptfs-wrap-passphrase /home/ubuntu/ab.sig
The program 'ecryptfs-wrap-passphrase' is currently not installed. You can install it by typing:
sudo apt-get install ecryptfs-utils
# (installed in another window)
ubuntu@panda-test:~$ ecryptfs-wrap-passphrase /home/ubuntu/ab.sig
Passphrase to wrap:
Wrapping passphrase:
ubuntu@panda-test:~$ ecryptfs-add-passphrase
Passphrase:
Inserted auth tok with sig [1830fcc608085939] into the user session keyring
ubuntu@panda-test:~$ mount.ecryptfs_private ab
fopen: No such file or directory
keyctl_search: Success
Perhaps try the interactive 'ecryptfs-mount-private'
ubuntu@panda-test:~$ dpkg-query -W ecryptfs-utils libecryptfs0
ecryptfs-utils 92-0ubuntu1
libecryptfs0 92-0ubuntu1

[Serge Hallyn (serge-hallyn)]

Quoting Robie Basak (<email address hidden>):
> No, I still can't get it to fail.
>
> ubuntu@panda-test:~$ mkdir /home/ubuntu/.ecryptfs
> ubuntu@panda-test:~$ mkdir /home/ubuntu/a /home/ubuntu/b
> ubuntu@panda-test:~$ cat > /home/ubuntu/.ecryptfs/ab.conf << EOF
> > /home/ubuntu/a /home/ubuntu/b ecryptfs none 0 0
> > EOF
> ubuntu@panda-test:~$ ecryptfs-wrap-passphrase /home/ubuntu/ab.sig
> The program 'ecryptfs-wrap-passphrase' is currently not installed. You can install it by typing:
> sudo apt-get install ecryptfs-utils
> # (installed in another window)
> ubuntu@panda-test:~$ ecryptfs-wrap-passphrase /home/ubuntu/ab.sig

sorry, that should be " ecryptfs-wrap-passphrase /home/ubuntu/.ecryptfs/ab.sig "

[Robie Basak (racb)]

This worked. Reproduced the bug in oneiric (evidently an infinite loop), the problem went away in -proposed. Normal ecryptfs operation (via ecryptfs-setup-private) also appears to be unaffected.

# dpkg -l|grep ecryptfs
ii ecryptfs-utils 92-0ubuntu1.1 ecryptfs cryptographic filesystem (utilities)
ii libecryptfs0 92-0ubuntu1.1 ecryptfs cryptographic filesystem (library)

[Serge Hallyn (serge-hallyn)]

Yay! Thanks, Robie, sorry about all the time that took.

[Launchpad Janitor (janitor)]

This bug was fixed in the package ecryptfs-utils - 92-0ubuntu1.1

---------------
ecryptfs-utils (92-0ubuntu1.1) oneiric-proposed; urgency=low

  [ Serge Hallyn ]
  * fix infinite loop on arm: fgetc returns an int, and -1 at end of
    options. Arm makes char unsigned. (LP: #884407)

  [ Michael Terry ]
  * debian/local/ecryptfs-verify, debian/rules:
    - Backport ecryptfs-verify from version 93. Required to support
      gnome-control-center's check to see if it should display
      the autologin controls. LP: #576133
 -- Michael Terry <email address hidden> Thu, 10 Nov 2011 10:33:01 -0500