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.