[U-Boot] [PULL] efi patch queue 2018-07-25

[Tom Rini <trini@konsulko.com>]

On Sat, Jul 28, 2018 at 11:32:56PM +0200, Heinrich Schuchardt wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 07/28/2018 08:33 PM, Tom Rini wrote:
> > On Sat, Jul 28, 2018 at 07:10:39PM +0200, Heinrich Schuchardt
> > wrote:
> >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
> >> 
> >> On 07/28/2018 06:32 PM, Tom Rini wrote:
> >>> On Sat, Jul 28, 2018 at 06:21:58PM +0200, Heinrich Schuchardt 
> >>> wrote:
> >>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
> >>>> 
> >>>> On 07/28/2018 06:13 PM, Tom Rini wrote:
> >>>>> On Sat, Jul 28, 2018 at 06:07:20PM +0200, Heinrich
> >>>>> Schuchardt wrote:
> >>>>> 
> >>>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
> >>>>>> 
> >>>>>> On 07/28/2018 05:55 PM, Tom Rini wrote:
> >>>>>>> On Wed, Jul 25, 2018 at 03:04:27PM +0200, Alexander
> >>>>>>> Graf wrote:
> >>>>>>> 
> >>>>>>>> Hi Tom,
> >>>>>>>> 
> >>>>>>>> This is my current patch queue for efi.  Please
> >>>>>>>> pull.
> >>>>>>>> 
> >>>>>>>> Alex
> >>>>>>>> 
> >>>>>>>> 
> >>>>>>>> The following changes since commit 
> >>>>>>>> 323a73adc9a1bf2de43fe03bdd9c3038ce7c2784:
> >>>>>>>> 
> >>>>>>>> mtd: nand: add new enum for storing ECC algorithm 
> >>>>>>>> (2018-07-23 14:33:21 -0400)
> >>>>>>>> 
> >>>>>>>> are available in the git repository at:
> >>>>>>>> 
> >>>>>>>> git://github.com/agraf/u-boot.git
> >>>>>>>> tags/signed-efi-next
> >>>>>>>> 
> >>>>>>>> for you to fetch changes up to 
> >>>>>>>> 0b8a88ab6aa24de0ef2bf1e8109409f71e770a8e:
> >>>>>>>> 
> >>>>>>>> MAINTAINERS: assign lib/charset.c (2018-07-25
> >>>>>>>> 15:00:24 +0200)
> >>>>>>>> 
> >>>>>>> 
> >>>>>>> NAK, this breaks one of the filesystem tests. 
> >>>>>>> Specifically: commit 
> >>>>>>> 0dc1bfb7302d220a48364263d5632d6d572b069b Author:
> >>>>>>> Heinrich Schuchardt <xypron.glpk@gmx.de> Date:   Mon
> >>>>>>> Jul 2 02:41:23 2018 +0200
> >>>>>>> 
> >>>>>>> fs: fat: cannot write to subdirectories
> >>>>>>> 
> >>>>>>> Breaks TC13: 1MB write to ./1MB.file.w2
> >>>>>>> 
> >>>>>> 
> >>>>>> Hello Tom,
> >>>>>> 
> >>>>>> please, provide the link to the Travis log with the
> >>>>>> failure.
> >>>>> 
> >>>>> It's actually not in travis.  Running test/fs/fs-test.sh is
> >>>>>  annoying to automate: FSTST=`./test/fs/fs-test.sh 2>&1 |
> >>>>> tail -n 3 | head -n 1` echo $FSTST | grep -q "TOTAL PASS:
> >>>>> 204 TOTAL FAIL: 12" && exit 0 || exit 1
> >>>>> 
> >>>>> but I should see if I can get that into .travis.yml.
> >>>>> 
> >>>> 
> >>>> ./test/fs/fs-test.sh Missing mkfs binary. Exiting!
> >>>> 
> >>>> You wouldn't run tests as root? Is this test meant to be run 
> >>>> with fakeroot?
> >>> 
> >>> It requires sudo to work along with various utilities to make
> >>> the various filesystems.
> >>> 
> >> 
> >> Tom please, have a look at the files created by the tests w/o my
> >> patch.
> >> 
> >> This is what the find command returns:
> >> 
> >> sandbox/test/fs/mnt sandbox/test/fs/mnt/SUBDIR 
> >> sandbox/test/fs/mnt/2.5GB.file sandbox/test/fs/mnt/1MB.file 
> >> sandbox/test/fs/mnt/1MB.file.w sandbox/test/fs/mnt/1MB.file.w2 
> >> sandbox/test/fs/mnt/./1MB.file.w2
> >> 
> >> You observe that the last file has an illegal file name (yes,
> >> the filename itself is "./1MB.file.w2". It should never have been
> >> created.
> >> 
> >> Without my patch this illegal file is not created.
> >> 
> >> Why should this be a reason to dismiss my patch?
> > 
> > Ah, OK, thanks for looking.  Please submit a patch that updates
> > the tests.
> > 
> 
> With Takahiro's patch series
> 
> fs: fat: extend FAT write operations
> https://patchwork.ozlabs.org/project/uboot/list/?series=56580
> https://lists.denx.de/pipermail/u-boot/2018-July/335683.html
> 
> the FAT driver will finally correctly support paths with subdirectories.
> 
> With that patch series the created files are:
> 
> sandbox/test/fs/mnt
> sandbox/test/fs/mnt/SUBDIR
> sandbox/test/fs/mnt/2.5GB.file
> sandbox/test/fs/mnt/1MB.file
> sandbox/test/fs/mnt/1MB.file.w
> sandbox/test/fs/mnt/1MB.file.w2
> 
> There is nothing wrong with the TC13 test. After writing it tries to
> do the verification with (b) and without (c) a relative path. If both
> subtests are passed the file system is working as expected. And as you
> already will have observed TC13b and TC13c are not passed without
> Takahiro's patch series.

OK, I'm coming back in here.  The log file from running the test is
sandbox/test/fs/fs-test.fs.fat32.out_clean and if we look for TC13, this
is what we see:
=> # Write it via "same directory", i.e. "." dirent
=> # Test Case 13a - Check directory traversal
=> save host 0:0 0x01000008 ./1MB.file.w2 $filesize
FAT: illegal filename (./1MB.file.w2)

So the change is leading to us not being able to write "./1MB.file.w2"
at all, rather than (as the test was intended to catch!) writing
"1MB.file.w2" to ".".  Yes, the current code is wrong as it writes the
illegal file "./1MB.file.w2" but the new behavior is wrong too.  I am
going to take the EFI PR as-is and I'm going to push a commit that
updates the expected results and explains why.  I'm going to leave it to
you and Takahiro-san's series to address the problem and allow for
"./file" to write "file" to "." as intended.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180730/baba4341/attachment.sig>