11 Feb, 2020

1 commit


08 Feb, 2020

2 commits

  • The code for handing file overwrite incorrectly calculated the amount of
    data to write when writing to the last non-cluster aligned chunk. Fix
    this by ensuring that no more data than the 'filesize' is written to disk.
    While touching min()-based calculations, change it to type-safe min_t()
    function.

    Signed-off-by: Marek Szyprowski

    This patch finally fixes the issue revealed by the test script from the
    previous patch. The correctness of the change has been also verified by
    the following additional test scripts:

    --->8-fat_test2.sh---
    #!/bin/bash
    make sandbox_defconfig
    make
    dd if=/dev/zero of=/tmp/10M.img bs=1024 count=10k
    mkfs.vfat -v /tmp/10M.img
    cat >/tmp/cmds </tmp/model/file0001.raw
    yes ABC | head -c 4096 >/tmp/model/file0003.raw
    yes ABC | head -c 4096 >/tmp/model/file0005.raw
    yes DEF | head -c 7936 >/tmp/model/file0007.raw
    mcopy -n -i /tmp/10M.img ::file0001.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0003.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0005.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0007.raw /tmp/result
    hd /tmp/10M.img
    if diff -urq /tmp/model /tmp/result
    then
    echo Test okay
    else
    echo Test fail
    fi
    --->8-fat_test3.sh---
    #!/bin/bash
    make sandbox_defconfig
    make
    dd if=/dev/zero of=/tmp/10M.img bs=1024 count=10k
    mkfs.vfat -v /tmp/10M.img
    cat >/tmp/cmds </tmp/model/file0001.raw
    yes ABC | head -c 4096 >/tmp/model/file0003.raw
    yes ABC | head -c 4096 >/tmp/model/file0005.raw
    yes DEF | head -c 8448 >/tmp/model/file0007.raw
    mcopy -n -i /tmp/10M.img ::file0001.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0003.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0005.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0007.raw /tmp/result
    hd /tmp/10M.img
    if diff -urq /tmp/model /tmp/result
    then
    echo Test okay
    else
    echo Test fail
    fi
    --->8-fat_test4.sh---
    #!/bin/bash
    make sandbox_defconfig
    make
    dd if=/dev/zero of=/tmp/10M.img bs=1024 count=10k
    mkfs.vfat -v /tmp/10M.img
    cat >/tmp/cmds </tmp/model/file0001.raw
    yes ABC | head -c 4096 >/tmp/model/file0003.raw
    yes ABC | head -c 4096 >/tmp/model/file0005.raw
    yes DEF | head -c 2304 >/tmp/model/file0007.raw
    yes GHI | head -c 2304 >>/tmp/model/file0007.raw
    yes DEF | head -c 2304 >>/tmp/model/file0007.raw
    yes GHI | head -c 2304 >>/tmp/model/file0007.raw
    mcopy -n -i /tmp/10M.img ::file0001.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0003.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0005.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0007.raw /tmp/result
    hd /tmp/10M.img
    if diff -urq /tmp/model /tmp/result
    then
    echo Test okay
    else
    echo Test fail
    fi
    --->8---
    Feel free to prepare a proper sandbox/py_test based tests based on
    the provided test scripts.

    Marek Szyprowski
     
  • The code for handing file overwrite incorrectly assumed that the file on
    disk is always contiguous. This resulted in corrupting disk structure
    every time when write to existing fragmented file happened. Fix this
    by adding proper check for cluster discontinuity and adjust chunk size
    on each partial write.

    Signed-off-by: Marek Szyprowski

    This patch partially fixes the issue revealed by the following test
    script:

    --->8-fat_test1.sh---
    #!/bin/bash
    make sandbox_defconfig
    make
    dd if=/dev/zero of=/tmp/10M.img bs=1024 count=10k
    mkfs.vfat -v /tmp/10M.img
    cat >/tmp/cmds </tmp/model/file0001.raw
    yes ABC | head -c 4096 >/tmp/model/file0003.raw
    yes ABC | head -c 4096 >/tmp/model/file0005.raw
    yes DEF | head -c 16384 >/tmp/model/file0007.raw
    mcopy -n -i /tmp/10M.img ::file0001.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0003.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0005.raw /tmp/result
    mcopy -n -i /tmp/10M.img ::file0007.raw /tmp/result
    hd /tmp/10M.img
    if diff -urq /tmp/model /tmp/result
    then
    echo Test okay
    else
    echo Test fail
    fi
    --->8---

    Overwritting a discontiguous test file (file0007.raw) no longer causes
    corruption to file0003.raw, which's data lies between the chunks of the
    test file. The amount of data written to disk is still incorrect, what
    causes damage to the file (file0005.raw), which's data lies next to the
    test file. This will be fixed by the next patch.

    Feel free to prepare a proper sandbox/py_test based tests based on the
    provided test scripts.

    Marek Szyprowski
     

06 Feb, 2020

1 commit

  • At present dm/device.h includes the linux-compatible features. This
    requires including linux/compat.h which in turn includes a lot of headers.
    One of these is malloc.h which we thus end up including in every file in
    U-Boot. Apart from the inefficiency of this, it is problematic for sandbox
    which needs to use the system malloc() in some files.

    Move the compatibility features into a separate header file.

    Signed-off-by: Simon Glass

    Simon Glass
     

05 Dec, 2019

1 commit

  • Unlink test for FAT file system seems to fail at test_unlink2.
    (When I added this test, I haven't seen any errors though.)
    for example,
    ===8 assert('0 file(s), 2 dir(s)' in output)
    E AssertionError: assert '0 file(s), 2 dir(s)' in ' ./\r\r\n ../\r\r\n 0 0123456789abcdef11\r\r\n\r\r\n1 file(s), 2 dir(s)'

    test/py/tests/test_fs/test_unlink.py:52: AssertionError
    ===>8===

    This can happen when fat_itr_next() wrongly detects an already-
    deleted directory entry.

    File deletion, which was added in the commit f8240ce95d64 ("fs: fat:
    support unlink"), is implemented by marking its entry for a short name
    with DELETED_FLAG, but related entry slots for a long file name are kept
    unmodified. (So entries will never be actually deleted from media.)

    To handle this case correctly, an additional check for a directory slot
    will be needed in fat_itr_next().

    In addition, I added extra comments about long file name and short file
    name format in FAT file system. Although they are not directly related
    to the issue, I hope it will be helpful for better understandings
    in general.

    Signed-off-by: AKASHI Takahiro

    AKASHI Takahiro
     

12 Oct, 2019

2 commits


26 Aug, 2019

1 commit

  • File was found on specified location. Info about file was read,
    but then immediately destroyed using 'free' call. As a result
    file size was set to 0, hence fat process didn't read any data.

    Premature 'free' call removed. Resources are freed right before
    function return. File is read correctly.

    Signed-off-by: Martin Vystrcil

    Martin Vystrčil
     

29 May, 2019

5 commits

  • Contrary to fat12/16, fat32 can have root directory at any location
    and its size can be expanded.
    Without this patch, root directory won't grow properly and so we will
    eventually fail to add files under root directory. Please note that this
    can happen even if you delete many files as deleted directory entries
    are not reclaimed but just marked as "deleted" under the current
    implementation.

    Signed-off-by: AKASHI Takahiro
    Tested-by: Heinrich Schuchardt

    AKASHI Takahiro
     
  • When a long name directory entry is created, multiple directory entries
    may be occupied across a directory cluster boundary. Since only one
    directory cluster is cached in a directory iterator, a first cluster must
    be written back to device before switching over a second cluster.

    Without this patch, some added files may be lost even if you don't see
    any failures on write operation.

    Signed-off-by: AKASHI Takahiro
    Tested-by: Heinrich Schuchardt

    AKASHI Takahiro
     
  • With the commit below, fat now correctly handles a file read under
    a non-cluster-aligned root directory of fat12/16.
    Write operation should be fixed in the same manner.

    Fixes: commit 9b18358dc05d ("fs: fat: fix reading non-cluster-aligned
    root directory")
    Signed-off-by: AKASHI Takahiro
    Cc: Anssi Hannula
    Tested-by: Heinrich Schuchardt

    AKASHI Takahiro
     
  • fat_itr_root() allocates fatbuf so we free it on the exit path, if
    the function fails we should not free it, check the return value
    and skip freeing if the function fails.

    Signed-off-by: Andrew F. Davis

    Andrew F. Davis
     
  • File names may not contain control characters (< 0x20).
    Simplify the coding.

    Signed-off-by: Heinrich Schuchardt

    Heinrich Schuchardt
     

10 Apr, 2019

1 commit

  • A FAT12/FAT16 root directory location is specified by a sector offset and
    it might not start at a cluster boundary. It also resides before the
    data area (before cluster 2).

    However, the current code assumes that the root directory is located at
    a beginning of a cluster, causing no files to be found if that is not
    the case.

    Since the FAT12/FAT16 root directory is located before the data area
    and is not aligned to clusters, using unsigned cluster numbers to refer
    to the root directory does not work well (the "cluster number" may be
    negative, and even allowing it be signed would not make it properly
    aligned).

    Modify the code to not use the normal cluster numbering when referring to
    the root directory of FAT12/FAT16 and instead use a cluster-sized
    offsets counted from the root directory start sector.

    This is a relatively common case as at least the filesystem formatter on
    Win7 seems to create such filesystems by default on 2GB USB sticks when
    "FAT" is selected (cluster size 64 sectors, rootdir size 32 sectors,
    rootdir starts at half a cluster before cluster 2).

    dosfstools mkfs.vfat does not seem to create affected filesystems.

    Signed-off-by: Anssi Hannula
    Reviewed-by: Bernhard Messerklinger
    Tested-by: Bernhard Messerklinger

    Anssi Hannula
     

01 Mar, 2019

1 commit

  • When compiling with DEBUG=1 an error
    fs/fat/fat_write.c:831: undefined reference to `__aeabi_ldivmod'
    occurred.

    We should use do_div() instead of the modulus operator.

    filesize and cur_pos cannot be negative. So let's use u64 to avoid
    warnings.

    Fixes: cb8af8af5ba0 ("fs: fat: support write with non-zero offset")
    Signed-off-by: Heinrich Schuchardt

    Heinrich Schuchardt
     

19 Feb, 2019

2 commits


01 Feb, 2019

1 commit


11 Jan, 2019

1 commit

  • This particular commit is causing a regression on stih410-b2260 and
    other platforms when reading from FAT16. Noting that I had rebased the
    original fix from Thomas onto then-current master, there is also
    question from Akashi-san if the change is still needed after other FAT
    fixes that have gone in.

    This reverts commit a68b0e11ea774492713a65d9fd5bb525fcaefff3.

    Reported-by: Patrice Chotard
    Cc: AKASHI Takahiro
    Cc: Thomas RIENOESSL
    Signed-off-by: Tom Rini

    Tom Rini
     

07 Dec, 2018

2 commits

  • The long name apparently can be accumulated using multiple
    13-byte slots. Unfortunately we never checked how many we
    can actually fit in the buffer we are reading to.

    Signed-off-by: Patrick Wildt

    Patrick Wildt
     
  • The cluster size specifies how many sectors make up a cluster. A
    cluster size of zero makes no sense, as it would mean that the
    cluster is made up of no sectors. This will later lead into a
    division by zero in sect_to_clust(), so better take care of that
    early.

    The MAX_CLUSTSIZE define can reduced using a define to make some
    room in low-memory system. Unfortunately if the code reads a
    filesystem with a bigger cluster size it will overflow the buffer.

    Signed-off-by: Patrick Wildt

    Patrick Wildt
     

21 Nov, 2018

1 commit


16 Oct, 2018

1 commit


07 Oct, 2018

2 commits


24 Sep, 2018

13 commits

  • The FAT driver supports unaligned reads and writes and EFI applications
    will make use of these. So a misaligned buffer is only worth a debug
    message.

    Signed-off-by: Heinrich Schuchardt
    Signed-off-by: Alexander Graf

    Heinrich Schuchardt
     
  • In this patch, unlink support is added to FAT file system.
    A directory can be deleted only if it is empty.

    In this implementation, only a directory entry for a short file name
    will be removed. So entries for a long file name can and should be
    reclaimed with fsck.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • In this patch, mkdir support is added to FAT file system.
    A newly created directory contains only "." and ".." entries.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • The starting cluster number of directory is needed to initialize ".."
    (parent directory) entry when creating a new directory.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • In this patch, all the necessary code for allowing for a file offset
    at write is implemented. What plays a major roll here is get_set_cluster(),
    which, in contrast to its counterpart, set_cluster(), only operates on
    already-allocated clusters, overwriting with data.

    So, with a file offset specified, set_contents() seeks and writes data
    with set_get_cluster() until the end of a file, and, once it reaches
    there, continues writing with set_cluster() for the rest.

    Please note that a file will be trimmed as a result of write operation if
    write ends before reaching file's end. This is an intended behavior
    in order to maintain compatibility with the current interface.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • The current write implementation is quite simple: remove existing clusters
    and then allocating new ones and filling them with data. This, inevitably,
    enforces always writing from the beginning of a file.

    As the first step to lift this restriction, fat_file_write() and
    set_contents() are modified to accept an additional parameter, file offset
    and further re-factored so that, in the next patch, all the necessary code
    will be put into set_contents().

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • In this patch, write implementation is overhauled and rewritten by
    making full use of directory iterator. The obvious bonus is that we are
    now able to write to a file with a directory path, like /A/B/C/FILE.

    Please note that, as there is no notion of "current directory" on u-boot,
    a file name specified must contain an absolute directory path. Otherwise,
    "/" (root directory) is assumed.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • It would be good that FAT write function return error code instead of
    just returning -1 as fat_read_file() does.
    This patch attempts to address this issue although it is 'best effort
    (or estimate)' for now.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • FAT file system's long file name support is a bit complicated and has some
    restrictions on its naming. We should be careful about it especially for
    write as it may easily end up with wrong file system.

    normalize_longname() check for the rules and normalize a file name
    if necessary. Please note, however, that this function is yet to be
    extended to fully comply with the standard.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • This reverts commit 0dc1bfb7302d220a48364263d5632d6d572b069b.
    The succeeding patch series will supersede it.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • In my attempt to re-work write operation, it was revealed that iterator's
    "clust" does not always point to a cluster to which a current directory
    entry ("dent") belongs.
    This patch assures that it is always true by adding "next_clust" which is
    used solely for dereferencing a cluster chain.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • FAT's root directory does not have "." nor ".."
    So care must be taken when scanning root directory with fat_itr_resolve().
    Without this patch, any file path starting with "." or ".." will not be
    resolved at all.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     
  • get_fs_info() was introduced in major re-work of read operation by Rob.
    We want to reuse this function in write operation by extending it with
    additional members in fsdata structure.

    Signed-off-by: AKASHI Takahiro
    Signed-off-by: Alexander Graf

    AKASHI Takahiro
     

20 Aug, 2018

1 commit


25 Jul, 2018

1 commit

  • fs_fat_write() is not able to write to subdirectories.

    Currently if a filepath with a leading slash is passed, the slash is
    treated as part of the filename to be created in the root directory.

    Strip leading (back-)slashes.

    Check that the remaining filename does not contain any illegal characters
    (<>:"/\|?*). This way we will throw an error when trying to write to a
    subdirectory.

    Signed-off-by: Heinrich Schuchardt
    Signed-off-by: Alexander Graf

    Heinrich Schuchardt