[PATCH] Added CONFIG_VFAT_FS_DUALNAMES option

This is a new patch for VFAT long filename support, replacing the one
that I posted last month. It retains a lot more functionality then the
previous patch.

A FAQ will be posted immediately after this patch to answer the
questions that were raised from the previous discussion.

Cheers, Tridge


------------

When VFAT_FS_DUALNAMES is disabled we avoid the creation of 8.3 short
filenames for files on VFAT filesystems that require a long name. The
patch uses a pattern of 11 bytes in the directory entry which contains
invalid characters such that it cannot be considered to be a valid short
filename.

Signed-off-by: Andrew Tridgell <tridge@...>
Acked-by: Dave Kleikamp <shaggy@...>
Acked-by: Paul E. McKenney <paulmck@...>
---
 fs/fat/Kconfig      |   20 +++++++++++++++++
 fs/fat/dir.c        |   15 ++++++-------
 fs/fat/namei_vfat.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 8 deletions(-)

diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 182f9ff..907a5de 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -74,6 +74,26 @@ config VFAT_FS
   To compile this as a module, choose M here: the module will be called
   vfat.
 
+config VFAT_FS_DUALNAMES
+ bool "VFAT dual names support"
+ depends on VFAT_FS
+ help
+  This option provides support for dual filenames on VFAT filesystems.
+  If this option is disabled then file creation will either put
+  a short (8.3) name or a long name on the file, but never both.
+  The field where a shortname would normally go is filled with
+  invalid characters such that it cannot be considered a valid
+  short filename.
+
+  That means that long filenames created with this option
+  disabled will not be accessible at all to operating systems
+  that do not understand the VFAT extensions.
+
+  Users considering enabling this option should consider the implications
+  of any patents that may exist on dual filenames in VFAT.
+
+  If unsure, say N
+
 config FAT_DEFAULT_CODEPAGE
  int "Default codepage for FAT"
  depends on MSDOS_FS || VFAT_FS
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 38ff75a..cd5d3ec 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -420,14 +420,13 @@ parse_record:
  }
  i += chl;
  }
- if (!last_u)
- continue;
-
- /* Compare shortname */
- bufuname[last_u] = 0x0000;
- len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
- if (fat_name_match(sbi, name, name_len, bufname, len))
- goto found;
+ if (last_u) {
+ /* Compare shortname */
+ bufuname[last_u] = 0x0000;
+ len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
+ if (fat_name_match(sbi, name, name_len, bufname, len))
+ goto found;
+ }
 
  if (nr_slots) {
  void *longname = unicode + FAT_MAX_UNI_CHARS;
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 73471b7..894f44d 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -22,6 +22,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/namei.h>
+#include <linux/random.h>
 #include "fat.h"
 
 /*
@@ -586,6 +587,59 @@ xlate_to_uni(const unsigned char *name, int len, unsigned char *outname,
  return 0;
 }
 
+#ifndef CONFIG_VFAT_FS_DUALNAMES
+/*
+ * build a 11 byte 8.3 buffer which is not a short filename. We want 11
+ * bytes which:
+ *    - will be seen as a constant string to all APIs on Linux and Windows
+ *    - cannot be matched with wildcard patterns
+ *    - cannot be used to access the file
+ *    - has a low probability of collision within a directory
+ *    - has an invalid 3 byte extension
+ *    - contains at least one non-space and non-nul byte
+ */
+static void vfat_build_dummy_83_buffer(struct inode *dir, char *msdos_name)
+{
+ u32 rand_num = random32() & 0x3FFFFFFF;
+ int i;
+
+ /* a value of zero would leave us with only nul and spaces,
+ * which would not work with older linux systems
+ */
+ if (rand_num == 0)
+ rand_num = 1;
+
+ /* we start with a space followed by nul as spaces at the
+ * start of an entry are trimmed in FAT, which means that
+ * starting the 11 bytes with 0x20 0x00 gives us a value which
+ * cannot be used to access the file. It also means that the
+ * value as seen from all Windows and Linux APIs is a constant
+ */
+ msdos_name[0]  = ' ';
+ msdos_name[1]  = 0;
+
+ /* we use / and 2 nul bytes for the extension. These are
+ * invalid in FAT and mean that utilities that show the
+ * directory show no extension, but still work via the long
+ * name for old Linux kernels
+ */
+ msdos_name[8]  = '/';
+ msdos_name[9]  = 0;
+ msdos_name[10] = 0;
+
+ /*
+ * fill the remaining 6 bytes with random invalid values
+ * This gives us a low collision rate, which means a low
+ * chance of problems with chkdsk.exe and WindowsXP
+ */
+ for (i = 2; i < 8; i++) {
+ msdos_name[i] = rand_num & 0x1F;
+ rand_num >>= 5;
+ }
+}
+#endif
+
+
 static int vfat_build_slots(struct inode *dir, const unsigned char *name,
     int len, int is_dir, int cluster,
     struct timespec *ts,
@@ -628,6 +682,11 @@ static int vfat_build_slots(struct inode *dir, const unsigned char *name,
  goto shortname;
  }
 
+#ifndef CONFIG_VFAT_FS_DUALNAMES
+ vfat_build_dummy_83_buffer(dir, msdos_name);
+ lcase = 0;
+#endif
+
  /* build the entry of long file name */
  cksum = fat_checksum(msdos_name);
 
--
1.6.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[H. Peter Anvin]

tridge@... wrote:
> This is a new patch for VFAT long filename support, replacing the one
> that I posted last month. It retains a lot more functionality then the
> previous patch.
>
> A FAQ will be posted immediately after this patch to answer the
> questions that were raised from the previous discussion.
>

From a purely technical perspective, there is a major advantage to this
patch: it creates a "pure VFAT" filesystem, without a spurious filename
alias which still occupies namespace, hiddenly, and therefore could
cause an ill-defined amount of problems.  UMSDOS at least had the
decency to not expose its shortnames to a longname-aware OS.

However, as such it really should be paired with a "don't even recognize
the shortname if a longname exists" option.

It's also questionable IMO if this shouldn't be another FAT superdriver,
just as we have VFAT, MS-DOS etc. we could have "purevfat".

        -hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

hpa wrote:
 > However, as such it really should be paired with a "don't even recognize
 > the shortname if a longname exists" option.

Right, we could just skip the 8.3 name matching and go straight to the
longname match in fat_search_long(). I wonder if anyone ever relies on
the 8.3 matching when a file has a long name on Linux?

 > It's also questionable IMO if this shouldn't be another FAT superdriver,
 > just as we have VFAT, MS-DOS etc. we could have "purevfat".

It would be only a few lines of code difference between the two
drivers - is it worth the maintainence overhead of splitting it up?

Maybe a mount option to ignore 8.3 names for files with long names
would be better? Perhaps even the default?

Cheers, Tridge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[OGAWA Hirofumi]

tridge@... writes:

> hpa wrote:
>  > However, as such it really should be paired with a "don't even recognize
>  > the shortname if a longname exists" option.
>
> Right, we could just skip the 8.3 name matching and go straight to the
> longname match in fat_search_long(). I wonder if anyone ever relies on
> the 8.3 matching when a file has a long name on Linux?

Wine or such is using it.

>  > It's also questionable IMO if this shouldn't be another FAT superdriver,
>  > just as we have VFAT, MS-DOS etc. we could have "purevfat".
>
> It would be only a few lines of code difference between the two
> drivers - is it worth the maintainence overhead of splitting it up?
>
> Maybe a mount option to ignore 8.3 names for files with long names
> would be better? Perhaps even the default?

It sounds like to be start of yet another UMSDOS with minimal one. I
wouldn't like to repeat UMSDOS history, i.e. I think it needs real users
and developers. If not, I wouldn't like to add.

Thanks.
--
OGAWA Hirofumi <hirofumi@...>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[OGAWA Hirofumi]

tridge@... writes:

> This is a new patch for VFAT long filename support, replacing the one
> that I posted last month. It retains a lot more functionality then the
> previous patch.
>
> A FAQ will be posted immediately after this patch to answer the
> questions that were raised from the previous discussion.

Looks good to me. It seems to be trying to be minimal change, and I
guess it would be worth a try for realworld problem.  Like FAQ is
saying, the default might be arguable though.

> When VFAT_FS_DUALNAMES is disabled we avoid the creation of 8.3 short
> filenames for files on VFAT filesystems that require a long name. The
> patch uses a pattern of 11 bytes in the directory entry which contains
> invalid characters such that it cannot be considered to be a valid short
> filename.
>
> Signed-off-by: Andrew Tridgell <tridge@...>
> Acked-by: Dave Kleikamp <shaggy@...>
> Acked-by: Paul E. McKenney <paulmck@...>
> ---
>  fs/fat/Kconfig      |   20 +++++++++++++++++
>  fs/fat/dir.c        |   15 ++++++-------
>  fs/fat/namei_vfat.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
> index 182f9ff..907a5de 100644
> --- a/fs/fat/Kconfig
> +++ b/fs/fat/Kconfig
> @@ -74,6 +74,26 @@ config VFAT_FS
>    To compile this as a module, choose M here: the module will be called
>    vfat.
>  
> +config VFAT_FS_DUALNAMES
> + bool "VFAT dual names support"
> + depends on VFAT_FS
> + help
> +  This option provides support for dual filenames on VFAT filesystems.
> +  If this option is disabled then file creation will either put
> +  a short (8.3) name or a long name on the file, but never both.
> +  The field where a shortname would normally go is filled with
> +  invalid characters such that it cannot be considered a valid
> +  short filename.
> +
> +  That means that long filenames created with this option
> +  disabled will not be accessible at all to operating systems
> +  that do not understand the VFAT extensions.
> +
> +  Users considering enabling this option should consider the implications
> +  of any patents that may exist on dual filenames in VFAT.
> +
> +  If unsure, say N
> +
>  config FAT_DEFAULT_CODEPAGE
>   int "Default codepage for FAT"
>   depends on MSDOS_FS || VFAT_FS
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 38ff75a..cd5d3ec 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -420,14 +420,13 @@ parse_record:
>   }
>   i += chl;
>   }
> - if (!last_u)
> - continue;
> -
> - /* Compare shortname */
> - bufuname[last_u] = 0x0000;
> - len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> - if (fat_name_match(sbi, name, name_len, bufname, len))
> - goto found;
> + if (last_u) {
> + /* Compare shortname */
> + bufuname[last_u] = 0x0000;
> + len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> + if (fat_name_match(sbi, name, name_len, bufname, len))
> + goto found;
> + }
>  
>   if (nr_slots) {
>   void *longname = unicode + FAT_MAX_UNI_CHARS;
> diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> index 73471b7..894f44d 100644
> --- a/fs/fat/namei_vfat.c
> +++ b/fs/fat/namei_vfat.c
> @@ -22,6 +22,7 @@
>  #include <linux/smp_lock.h>
>  #include <linux/buffer_head.h>
>  #include <linux/namei.h>
> +#include <linux/random.h>
>  #include "fat.h"
>  
>  /*
> @@ -586,6 +587,59 @@ xlate_to_uni(const unsigned char *name, int len, unsigned char *outname,
>   return 0;
>  }
>  
> +#ifndef CONFIG_VFAT_FS_DUALNAMES
> +/*
> + * build a 11 byte 8.3 buffer which is not a short filename. We want 11
> + * bytes which:
> + *    - will be seen as a constant string to all APIs on Linux and Windows
> + *    - cannot be matched with wildcard patterns
> + *    - cannot be used to access the file
> + *    - has a low probability of collision within a directory
> + *    - has an invalid 3 byte extension
> + *    - contains at least one non-space and non-nul byte
> + */
> +static void vfat_build_dummy_83_buffer(struct inode *dir, char *msdos_name)
> +{
> + u32 rand_num = random32() & 0x3FFFFFFF;
> + int i;
> +
> + /* a value of zero would leave us with only nul and spaces,
> + * which would not work with older linux systems
> + */
> + if (rand_num == 0)
> + rand_num = 1;
> +
> + /* we start with a space followed by nul as spaces at the
> + * start of an entry are trimmed in FAT, which means that
> + * starting the 11 bytes with 0x20 0x00 gives us a value which
> + * cannot be used to access the file. It also means that the
> + * value as seen from all Windows and Linux APIs is a constant
> + */
> + msdos_name[0]  = ' ';
> + msdos_name[1]  = 0;
> +
> + /* we use / and 2 nul bytes for the extension. These are
> + * invalid in FAT and mean that utilities that show the
> + * directory show no extension, but still work via the long
> + * name for old Linux kernels
> + */
> + msdos_name[8]  = '/';
> + msdos_name[9]  = 0;
> + msdos_name[10] = 0;
> +
> + /*
> + * fill the remaining 6 bytes with random invalid values
> + * This gives us a low collision rate, which means a low
> + * chance of problems with chkdsk.exe and WindowsXP
> + */
> + for (i = 2; i < 8; i++) {
> + msdos_name[i] = rand_num & 0x1F;
> + rand_num >>= 5;
> + }
> +}
> +#endif
> +
> +
>  static int vfat_build_slots(struct inode *dir, const unsigned char *name,
>      int len, int is_dir, int cluster,
>      struct timespec *ts,
> @@ -628,6 +682,11 @@ static int vfat_build_slots(struct inode *dir, const unsigned char *name,
>   goto shortname;
>   }
>  
> +#ifndef CONFIG_VFAT_FS_DUALNAMES
> + vfat_build_dummy_83_buffer(dir, msdos_name);
> + lcase = 0;
> +#endif
> +
>   /* build the entry of long file name */
>   cksum = fat_checksum(msdos_name);

--
OGAWA Hirofumi <hirofumi@...>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Jan Engelhardt-3]

On Sat, 27 Jun 2009 05:19:33 +1000, tridge@... wrote:
>
>When VFAT_FS_DUALNAMES is disabled we avoid the creation of 8.3
>short filenames for files on VFAT filesystems that require a long
>name. The patch uses a pattern of 11 bytes in the directory entry
>which contains invalid characters such that it cannot be considered
>to be a valid short filename.

Can't we make this a runtime option?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Jan Engelhardt-3]

On Sat, 27 Jun 2009 10:48:25 +0900, OGAWA Hirofumi wrote:

>It sounds like to be start of yet another UMSDOS with minimal one. I
>wouldn't like to repeat UMSDOS history, i.e. I think it needs real
>users and developers. If not, I wouldn't like to add.

And in that regard, there is http://posixovl.sf.net/ for everybody who
needs UMSDOS functionality.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Eric W. Biederman]

tridge@... writes:

> This is a new patch for VFAT long filename support, replacing the one
> that I posted last month. It retains a lot more functionality then the
> previous patch.
>
> A FAQ will be posted immediately after this patch to answer the
> questions that were raised from the previous discussion.
>
> Cheers, Tridge
>
>
> ------------
>
> When VFAT_FS_DUALNAMES is disabled we avoid the creation of 8.3 short
> filenames for files on VFAT filesystems that require a long name. The
> patch uses a pattern of 11 bytes in the directory entry which contains
> invalid characters such that it cannot be considered to be a valid short
> filename.
>
> Signed-off-by: Andrew Tridgell <tridge@...>
> Acked-by: Dave Kleikamp <shaggy@...>
> Acked-by: Paul E. McKenney <paulmck@...>
> ---
>  fs/fat/Kconfig      |   20 +++++++++++++++++
>  fs/fat/dir.c        |   15 ++++++-------
>  fs/fat/namei_vfat.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
> index 182f9ff..907a5de 100644
> --- a/fs/fat/Kconfig
> +++ b/fs/fat/Kconfig
> @@ -74,6 +74,26 @@ config VFAT_FS
>    To compile this as a module, choose M here: the module will be called
>    vfat.
>  
> +config VFAT_FS_DUALNAMES
> + bool "VFAT dual names support"
> + depends on VFAT_FS
> + help
> +  This option provides support for dual filenames on VFAT filesystems.
> +  If this option is disabled then file creation will either put
> +  a short (8.3) name or a long name on the file, but never both.
> +  The field where a shortname would normally go is filled with
> +  invalid characters such that it cannot be considered a valid
> +  short filename.
> +
> +  That means that long filenames created with this option
> +  disabled will not be accessible at all to operating systems
> +  that do not understand the VFAT extensions.
> +
> +  Users considering enabling this option should consider the implications
> +  of any patents that may exist on dual filenames in VFAT.
> +
> +  If unsure, say N
> +
>  config FAT_DEFAULT_CODEPAGE
>   int "Default codepage for FAT"
>   depends on MSDOS_FS || VFAT_FS
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 38ff75a..cd5d3ec 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -420,14 +420,13 @@ parse_record:
>   }
>   i += chl;
>   }
> - if (!last_u)
> - continue;
> -
> - /* Compare shortname */
> - bufuname[last_u] = 0x0000;
> - len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> - if (fat_name_match(sbi, name, name_len, bufname, len))
> - goto found;
> + if (last_u) {
> + /* Compare shortname */
> + bufuname[last_u] = 0x0000;
> + len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> + if (fat_name_match(sbi, name, name_len, bufname, len))
> + goto found;
> + }
>  
>   if (nr_slots) {
>   void *longname = unicode + FAT_MAX_UNI_CHARS;

This hunk allowing the examination of the long name if there is not a
short name is something else entirely.

You don't describe it in your summary, and don't make the case that
this is the correct behavior.

This should probably be split out into a separate patch if it is the
right thing to do, or drop it if (as it looks) there is no point in
that change.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Hi Eric,

 > This hunk allowing the examination of the long name if there is not a
 > short name is something else entirely.
 >
 > You don't describe it in your summary, and don't make the case that
 > this is the correct behavior.

Sorry for not explaining that part of the patch in my posting.

The reason I initially included it was that during the development of
this patch we were experimenting with variations of the
vfat_build_dummy_83_buffer() function which put a wide varity of
different values in the 11 bytes where a short filename would normally
go.

The most technically attractive varient was to put 11 spaces in
there. That has the advantage that Windows then returns the long name
to the GetShortPathName() API call, which is also what Windows does on
a NTFS filesystem when the NTFS filesystem is configured not to have
8.3 names. So it would have been very nice to have this same behaviour
on VFAT, instead of getting a single space back from
GetShortPathName() as we get with the patch I posted.

The reason we didn't go with the 11 spaces varient is that it causes
WindowsXP to bluescreen. This is just a bug in WinXP, and is not
present in Vista or Windows7.

The 2nd technical problem with 11 spaces is that current Linux kernels
would not see the long names, as the last_u logic in fat_search_long()
will skip files where the the 11 bytes don't have at least one
non-space byte before the first nul byte in either the 8 byte prefix
or the 3 byte extension.

The last_u logic is at odds with the behaviour of other operating
systems for VFAT. On all windows varients that I have tested (and on
MacOSX), the long name is still used no matter what values are in the
11 bytes. You can stick all nul bytes, all spaces, or anything else
you like in there and Windows will still allow you to operate on the
file by the long name and the filesystem will work perfectly.

So that part of the patch is bringing us inline with the behaviour of
other VFAT implementations.

More importantly, it gives us the opportunity in the future to move to
the techically better 11 spaces solution when either Microsoft fixes
their WindowsXP crash bug, or WindowsXP has such a low market share
that nobody cares any more (say like WfWg and maybe Win95 has now).

If we don't put in this change now then we won't be able to move to
the technically better solution later as we wouldn't have given Linux
distros a chance to include support for the 11 spaces in distros. By
putting that change in now, when (if?) we change to 11 spaces later we
will not be breaking recent Linux kernels.

 > This should probably be split out into a separate patch if it is the
 > right thing to do, or drop it if (as it looks) there is no point in
 > that change.

hopefully the above explanation makes it clear why the last_u change
should go together with the vfat_build_slots() change.

Cheers, Tridge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Hi Jan,

 > Can't we make this a runtime option?

John may be able to give you a more detailed answer, but the short
answer is that it is much safer legally to not have the code in the
binary kernel image at all.

Cheers, Tridge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Eric W. Biederman]

tridge@... writes:

> Hi Eric,
>
>  > This hunk allowing the examination of the long name if there is not a
>  > short name is something else entirely.
>  >
>  > You don't describe it in your summary, and don't make the case that
>  > this is the correct behavior.
>
> Sorry for not explaining that part of the patch in my posting.
>
> The reason I initially included it was that during the development of
> this patch we were experimenting with variations of the
> vfat_build_dummy_83_buffer() function which put a wide varity of
> different values in the 11 bytes where a short filename would normally
> go.
>
> The most technically attractive varient was to put 11 spaces in
> there. That has the advantage that Windows then returns the long name
> to the GetShortPathName() API call, which is also what Windows does on
> a NTFS filesystem when the NTFS filesystem is configured not to have
> 8.3 names. So it would have been very nice to have this same behaviour
> on VFAT, instead of getting a single space back from
> GetShortPathName() as we get with the patch I posted.
>
> The reason we didn't go with the 11 spaces varient is that it causes
> WindowsXP to bluescreen. This is just a bug in WinXP, and is not
> present in Vista or Windows7.
>
> The 2nd technical problem with 11 spaces is that current Linux kernels
> would not see the long names, as the last_u logic in fat_search_long()
> will skip files where the the 11 bytes don't have at least one
> non-space byte before the first nul byte in either the 8 byte prefix
> or the 3 byte extension.
>
> The last_u logic is at odds with the behaviour of other operating
> systems for VFAT. On all windows varients that I have tested (and on
> MacOSX), the long name is still used no matter what values are in the
> 11 bytes. You can stick all nul bytes, all spaces, or anything else
> you like in there and Windows will still allow you to operate on the
> file by the long name and the filesystem will work perfectly.
>
> So that part of the patch is bringing us inline with the behaviour of
> other VFAT implementations.
>
> More importantly, it gives us the opportunity in the future to move to
> the techically better 11 spaces solution when either Microsoft fixes
> their WindowsXP crash bug, or WindowsXP has such a low market share
> that nobody cares any more (say like WfWg and maybe Win95 has now).
>
> If we don't put in this change now then we won't be able to move to
> the technically better solution later as we wouldn't have given Linux
> distros a chance to include support for the 11 spaces in distros. By
> putting that change in now, when (if?) we change to 11 spaces later we
> will not be breaking recent Linux kernels.
>
>  > This should probably be split out into a separate patch if it is the
>  > right thing to do, or drop it if (as it looks) there is no point in
>  > that change.
>
> hopefully the above explanation makes it clear why the last_u change
> should go together with the vfat_build_slots() change.

It is part of the same investigation certainly.

Given what you have said our interpretation of vfat has a bug,
and that small change is a candidate for -stable.  If it could
be it's own patch.

That change seems much less controversial and obviously correct
than the other.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Hi Eric,

 > Given what you have said our interpretation of vfat has a bug,
 > and that small change is a candidate for -stable.  If it could
 > be it's own patch.

good point.

Hirofumi-san, would you support putting the last_u change into stable?

Cheers, Tridge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[OGAWA Hirofumi]

tridge@... writes:

>  > Given what you have said our interpretation of vfat has a bug,
>  > and that small change is a candidate for -stable.  If it could
>  > be it's own patch.
>
> good point.
>
> Hirofumi-san, would you support putting the last_u change into stable?

If you want, I have no problem to do. However, I'm not thinking that
part is a bug.  And -stable rule is also "a real bug that bothers
people", but there is even a no bug reporter which tell actual problem.

Thanks.
--
OGAWA Hirofumi <hirofumi@...>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Eric W. Biederman]

OGAWA Hirofumi <hirofumi@...> writes:

> tridge@... writes:
>
>>  > Given what you have said our interpretation of vfat has a bug,
>>  > and that small change is a candidate for -stable.  If it could
>>  > be it's own patch.
>>
>> good point.
>>
>> Hirofumi-san, would you support putting the last_u change into stable?
>
> If you want, I have no problem to do. However, I'm not thinking that
> part is a bug.  And -stable rule is also "a real bug that bothers
> people", but there is even a no bug reporter which tell actual problem.

Tridge.  Is there any reason to believe that Microsoft will continue
to treat Longfilenames without short filenames as valid in vfat?

If this turns into a contest of who can do the silliest things in
their vfat code microsoft could easily introduce a stricter directory
parser and cause all kinds of grief.

It wouldn't even surprise me if you haven't seen such shenanigans
while working on samba.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[James Bottomley-2]

On Sun, 2009-06-28 at 12:51 -0700, Eric W. Biederman wrote:

> OGAWA Hirofumi <hirofumi@...> writes:
>
> > tridge@... writes:
> >
> >>  > Given what you have said our interpretation of vfat has a bug,
> >>  > and that small change is a candidate for -stable.  If it could
> >>  > be it's own patch.
> >>
> >> good point.
> >>
> >> Hirofumi-san, would you support putting the last_u change into stable?
> >
> > If you want, I have no problem to do. However, I'm not thinking that
> > part is a bug.  And -stable rule is also "a real bug that bothers
> > people", but there is even a no bug reporter which tell actual problem.
>
>
> Tridge.  Is there any reason to believe that Microsoft will continue
> to treat Longfilenames without short filenames as valid in vfat?
>
> If this turns into a contest of who can do the silliest things in
> their vfat code microsoft could easily introduce a stricter directory
> parser and cause all kinds of grief.
>
> It wouldn't even surprise me if you haven't seen such shenanigans
> while working on samba.

If you own the platform, like Microsoft does, there are many things you
*could* do to make life difficult for others (like shipping a slightly
incompatible version of java, for instance ...).  However, having had
one or two consumer and regulatory backlashes from shipping updates
primarily designed to hobble what you think of as a competitor, you tend
to be much more wary about doing it so openly ... particularly when
you're trying to convince a wary customer base that you're the champion
of interoperability nowadays.

The bottom line is that we need to consider the current patch on its
merits for evading the vfat patent.  Speculating about what Microsoft
might or might not do to retaliate doesn't really help with this.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Eric W. Biederman]

James Bottomley <James.Bottomley@...> writes:

> On Sun, 2009-06-28 at 12:51 -0700, Eric W. Biederman wrote:
>> OGAWA Hirofumi <hirofumi@...> writes:
>>
>> > tridge@... writes:
>> >
>> >>  > Given what you have said our interpretation of vfat has a bug,
>> >>  > and that small change is a candidate for -stable.  If it could
>> >>  > be it's own patch.
>> >>
>> >> good point.
>> >>
>> >> Hirofumi-san, would you support putting the last_u change into stable?
>> >
>> > If you want, I have no problem to do. However, I'm not thinking that
>> > part is a bug.  And -stable rule is also "a real bug that bothers
>> > people", but there is even a no bug reporter which tell actual problem.
>>
>>
>> Tridge.  Is there any reason to believe that Microsoft will continue
>> to treat Longfilenames without short filenames as valid in vfat?
>>
>> If this turns into a contest of who can do the silliest things in
>> their vfat code microsoft could easily introduce a stricter directory
>> parser and cause all kinds of grief.
>>
>> It wouldn't even surprise me if you haven't seen such shenanigans
>> while working on samba.
>
> If you own the platform, like Microsoft does, there are many things you
> *could* do to make life difficult for others (like shipping a slightly
> incompatible version of java, for instance ...).  However, having had
> one or two consumer and regulatory backlashes from shipping updates
> primarily designed to hobble what you think of as a competitor, you tend
> to be much more wary about doing it so openly ... particularly when
> you're trying to convince a wary customer base that you're the champion
> of interoperability nowadays.
>
> The bottom line is that we need to consider the current patch on its
> merits for evading the vfat patent.  Speculating about what Microsoft
> might or might not do to retaliate doesn't really help with this.

This is a technical question.  Are we really in spec with the patch?

If we are not in spec. If we break things like checkdisk.exe.   Microsoft
can legitimately say we broke things and take technical measures to avoid
broken filesystems.  We have dome similar things to close source binary
modules.

My impression is that this patch puts us on the hairy edge.  If no
one is generating files without a short filename now.  That may cause
problems.

All else being equal this is a technically problematic patch as it
reduces the chances of interoperability.  I am just trying to gage
what those chances are.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Hi Eric,

 > Tridge.  Is there any reason to believe that Microsoft will continue
 > to treat Longfilenames without short filenames as valid in vfat?

I'd be quite surprised if they deliberately changed their VFAT code to
break Linux with this patch. I'd say it is more likely that once Linux
kernels with this change are in widespread use that Microsoft will
start to test any changes in their VFAT filesystem to make sure it
works with Linux with this patch.

 > It wouldn't even surprise me if you haven't seen such shenanigans
 > while working on samba.

In recent times Microsoft has been testing new OS releases against
Samba, and has been actively working with the Samba Team to try to
prevent breakages with new releases. That doesn't mean the scenario
you describe is impossible, it just means it isn't as likely as
perhaps it once was.

Cheers, Tridge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[James Bottomley-2]

On Sun, 2009-06-28 at 13:45 -0700, Eric W. Biederman wrote:

> James Bottomley <James.Bottomley@...> writes:
>
> > On Sun, 2009-06-28 at 12:51 -0700, Eric W. Biederman wrote:
> >> OGAWA Hirofumi <hirofumi@...> writes:
> >>
> >> > tridge@... writes:
> >> >
> >> >>  > Given what you have said our interpretation of vfat has a bug,
> >> >>  > and that small change is a candidate for -stable.  If it could
> >> >>  > be it's own patch.
> >> >>
> >> >> good point.
> >> >>
> >> >> Hirofumi-san, would you support putting the last_u change into stable?
> >> >
> >> > If you want, I have no problem to do. However, I'm not thinking that
> >> > part is a bug.  And -stable rule is also "a real bug that bothers
> >> > people", but there is even a no bug reporter which tell actual problem.
> >>
> >>
> >> Tridge.  Is there any reason to believe that Microsoft will continue
> >> to treat Longfilenames without short filenames as valid in vfat?
> >>
> >> If this turns into a contest of who can do the silliest things in
> >> their vfat code microsoft could easily introduce a stricter directory
> >> parser and cause all kinds of grief.
> >>
> >> It wouldn't even surprise me if you haven't seen such shenanigans
> >> while working on samba.
> >
> > If you own the platform, like Microsoft does, there are many things you
> > *could* do to make life difficult for others (like shipping a slightly
> > incompatible version of java, for instance ...).  However, having had
> > one or two consumer and regulatory backlashes from shipping updates
> > primarily designed to hobble what you think of as a competitor, you tend
> > to be much more wary about doing it so openly ... particularly when
> > you're trying to convince a wary customer base that you're the champion
> > of interoperability nowadays.
> >
> > The bottom line is that we need to consider the current patch on its
> > merits for evading the vfat patent.  Speculating about what Microsoft
> > might or might not do to retaliate doesn't really help with this.
>
> This is a technical question.  Are we really in spec with the patch?

Yes, the spec is here:

http://www.microsoft.com/whdc/system/platform/firmware/fatgen.mspx

(sorry, nasty licence agreement required to read it).

It says that long filenames *may* be created without short ones (but
that if you do this you'll have backwards compatibility problems on FAT
rather than FAT32 systems, which has been explicitly noted: after this
patch you can't read created files on 8.3 FAT).

> If we are not in spec. If we break things like checkdisk.exe.   Microsoft
> can legitimately say we broke things and take technical measures to avoid
> broken filesystems.  We have dome similar things to close source binary
> modules.

The question about spec isn't really that relevant with MS ... as you
can see from the Windows XP problems, they don't follow their own
spec ... the real question is has this been tested against existing
Windows versions; the answer is Yes (see FAQ).

> My impression is that this patch puts us on the hairy edge.  If no
> one is generating files without a short filename now.  That may cause
> problems.

Only for backwards compatibility ... and IBM has explicitly checked that
according to the FAQ.  They actually found Windows XP to be out of spec
(crashed with empty short filename) which is why the patch places an
invisible string in there now.

> All else being equal this is a technically problematic patch as it
> reduces the chances of interoperability.  I am just trying to gage
> what those chances are.

So the patch has been tested with Vista, Windows 7 and Windows XP ...
I'm sure IBM would be willing to do further interop testing if you
request.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Jamie Lokier]

tridge@... wrote:
> Hi Jan,
>
>  > Can't we make this a runtime option?
>
> John may be able to give you a more detailed answer, but the short
> answer is that it is much safer legally to not have the code in the
> binary kernel image at all.

Understood.

For those of us who don't want to omit the code, because we like
compatibility and we're not in affected countries, or for research, it
would be useful to have it as a mount option.

So there should be these compile-time options:

    1. CONFIG_VFAT_FS_DUALNAMES disabled:  Don't create shortnames.

    2. CONFIG_VFAT_FS_DUALNAMES enabled: Create shortnames, unless
       mount option "dualnames=no" is given, in which case that mount
       behaves as if CONFIG_VFAT_FS_DUALNAMES is disabled.

-- Jamie
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Jan Engelhardt-3]

On Sunday 2009-06-28 23:57, Jamie Lokier wrote:
>For those of us who don't want to omit the code, because we like
>compatibility and we're not in affected countries, or for research, it
>would be useful to have it as a mount option.
>
>So there should be these compile-time options:
>    2. CONFIG_VFAT_FS_DUALNAMES enabled: Create shortnames, unless
>       mount option "dualnames=no" is given, in which case that mount
>       behaves as if CONFIG_VFAT_FS_DUALNAMES is disabled.

If you are not in an affected country, why would you even want
to disable dualnames? [ on a per-mount basis...]

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/