Bug 106703 - [msdosfs] [patch] vn_stat() fails with files > 2Gb on msdosfs (non 386)
Summary: [msdosfs] [patch] vn_stat() fails with files > 2Gb on msdosfs (non 386)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 6.2-PRERELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Craig Rodrigues
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-12-14 06:40 UTC by Axel Gonzalez
Modified: 2007-01-29 02:20 UTC (History)
0 users

See Also:


Attachments
file.diff (534 bytes, patch)
2006-12-14 06:40 UTC, Axel Gonzalez
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Axel Gonzalez 2006-12-14 06:40:12 UTC
When you copy/create a file > 2Gb on a msdosfs, it is created correctly, but vn_stat() fails on the file.

Since the function is used on several userland programs (ls, rm), it seems like the file is not on the FS.

$ ls
ls: DVD.iso: Value too large to be stored in data type.


This is caused by an uncasted macro, instead of reporting the corrected size, it reports a negative one (or a very large one)

This problem is only on non-386 systems (which use alternate macro)

Fix: Apply the attached patch, recompile

sys_fs_msdosfs_bpb.h.patch



Patch attached with submission follows:
How-To-Repeat: Copy (create) a file > 2Gb on a msdosfs

try to ls (rm, stat, etc)

$ cp /ufs_disk/DVD.iso /msdosfs_disk
$ ls
ls: DVD.iso: Value too large to be stored in data type
Comment 1 Remko Lodder 2006-12-14 07:06:53 UTC
On Thu, Dec 14, 2006 at 06:30:15AM +0000, Axel Gonzalez wrote:
> 
> >Description:
> When you copy/create a file > 2Gb on a msdosfs, it is created correctly, but vn_stat() fails on the file.
> 
> Since the function is used on several userland programs (ls, rm), it seems like the file is not on the FS.
> 

Hello,

	first of all thanks for the report to help make FreeBSD better!

	One thing that crosses my mind is that (in my believing) the maximum
	size of a file under msdos(fs) was 2gb, which could explain this
	"problem" you are seeing. If that is still accurate, I don't think
	we should patch the file you mention, but refuse to accept files
	larger then 2gb since they will then not be visible at all on the
	disk when msdos itself is being used (or some other OS that reads
	out the msdosfs).

	I copied in Tom Rhodes for more clarification (he maintains msdosfs).

	Again thanks for the report and taking the time to mention this!

	Cheers,
	remko
-- 
Kind regards,

     Remko Lodder               ** remko@elvandar.org
     FreeBSD                    ** remko@FreeBSD.org

     /* Quis custodiet ipsos custodes */
Comment 2 Tom Rhodes freebsd_committer freebsd_triage 2006-12-15 00:48:48 UTC
On Thu, 14 Dec 2006 08:06:53 +0100
Remko Lodder <remko@elvandar.org> wrote:

> On Thu, Dec 14, 2006 at 06:30:15AM +0000, Axel Gonzalez wrote:
> > 
> > >Description:
> > When you copy/create a file > 2Gb on a msdosfs, it is created correctly, but vn_stat() fails on the file.
> > 
> > Since the function is used on several userland programs (ls, rm), it seems like the file is not on the FS.
> > 
> 
> Hello,
> 
> 	first of all thanks for the report to help make FreeBSD better!
> 
> 	One thing that crosses my mind is that (in my believing) the maximum
> 	size of a file under msdos(fs) was 2gb, which could explain this
> 	"problem" you are seeing. If that is still accurate, I don't think
> 	we should patch the file you mention, but refuse to accept files
> 	larger then 2gb since they will then not be visible at all on the
> 	disk when msdos itself is being used (or some other OS that reads
> 	out the msdosfs).
> 
> 	I copied in Tom Rhodes for more clarification (he maintains msdosfs).
> 
> 	Again thanks for the report and taking the time to mention this!
> 
> 	Cheers,
> 	remko

Hmm, that is an interesting problem, and I'm digging the fix.
FAT32 should handle file sizes up to (2^32)-1 bytes (one byte
fewer than four gigabytes.  So we should properly handle this
for all FAT32 file systems.

Heh, I don't want maintainership of MSDOSFS, can I give it
back or sell it to someone?  It's rarely used, I've only touched
it a few times.  :P

-- 
Tom Rhodes
Comment 3 Axel Gonzalez 2006-12-15 02:21:16 UTC
> > 	One thing that crosses my mind is that (in my believing) the maximum
> > 	size of a file under msdos(fs) was 2gb, which could explain this
> > 	"problem" you are seeing. If that is still accurate,

msdosfs_vnops.c:#define DOS_FILESIZE_MAX        0xffffffff
(4294967295) = 2^32 -1

cp works ok, after patching my kernel, the file is correct (md5 match).



>
> Hmm, that is an interesting problem, and I'm digging the fix.
> FAT32 should handle file sizes up to (2^32)-1 bytes (one byte
> fewer than four gigabytes.  So we should properly handle this
> for all FAT32 file systems.

I traced the problem down to the getulong() macro (non 386 version).

Problem is not with handling of the file, clusters, dirs. Just that 
msdosfs_getattr() reports an incorrect (signed) size, and it makes vn_stat() 
fail.

If it helps, more info:

MAX_OFF:9223372036854775807
(max size of any file)

size of the file:
Correct (ufs): 
3015487488 0xB3BCB000

msdosfs_getattr():
reported by w/o patch: 
18446744072430071808 0xFFFFFFFEB3BCB000
(this is why it returns EOVERFLOW)

reported with patch:
3015487488 0xB3BCB000
Comment 4 dfilter service freebsd_committer freebsd_triage 2006-12-19 01:56:15 UTC
rodrigc     2006-12-19 01:55:45 UTC

  FreeBSD src repository

  Modified files:
    sys/fs/msdosfs       bpb.h 
  Log:
  Fix get_ulong() macro on AMD64 (or any little-endian 64-bit platform).
  This bug caused vn_stat() to fail on files larger than 2gb on msdosfs
  filesystems on AMD64.
  
  PR:             106703
  Tested by:      Axel Gonzalez <loox e-shell net>
  MFC after:      3 days
  
  Revision  Changes    Path
  1.12      +1 -5      src/sys/fs/msdosfs/bpb.h
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 5 dfilter service freebsd_committer freebsd_triage 2006-12-19 02:32:15 UTC
rodrigc     2006-12-19 02:31:58 UTC

  FreeBSD src repository

  Modified files:
    sys/fs/msdosfs       bpb.h 
  Log:
  For big-endian version of getulong() macro, cast result to u_int32_t.
  This macro was written expecting a 32-bit unsigned long, and
  doesn't work properly on 64-bit systems.  This bug caused vn_stat()
  to return incorrect values for files larger than 2gb on msdosfs filesystems
  on 64-bit systems.
  
  PR:             106703
  Submitted by:   Axel Gonzalez <loox e-shell net>
  MFC after:      3 days
  
  Revision  Changes    Path
  1.13      +1 -1      src/sys/fs/msdosfs/bpb.h
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 6 Remko Lodder freebsd_committer freebsd_triage 2006-12-29 20:45:08 UTC
Responsible Changed
From-To: freebsd-bugs->trhodes

assign to tom
Comment 7 dfilter service freebsd_committer freebsd_triage 2007-01-05 05:50:53 UTC
rodrigc     2007-01-05 05:50:36 UTC

  FreeBSD src repository

  Modified files:        (Branch: RELENG_6)
    sys/fs/msdosfs       bpb.h 
  Log:
  MFC 1.14, 1.15:
  marcel      2006-12-21 05:40:46 UTC
  
    Unbreak 64-bit little-endian systems that do require alignment.
    The fix involves using le16dec(), le32dec(), le16enc() and
    le32enc(). This eliminates invalid casts and duplicated logic.
  
  PR:             106703
  Tested by:      Axel Gonzalez <loox e-shell net>
  
  Revision  Changes    Path
  1.11.2.1  +5 -26     src/sys/fs/msdosfs/bpb.h
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 8 Craig Rodrigues freebsd_committer freebsd_triage 2007-01-29 02:19:36 UTC
State Changed
From-To: open->closed

Fix committed to HEAD and RELENG_6. 


Comment 9 Craig Rodrigues freebsd_committer freebsd_triage 2007-01-29 02:19:36 UTC
Responsible Changed
From-To: trhodes->rodrigc

Fix committed to HEAD and RELENG_6.