Bug 78987 - [udf] [patch] udf fs: readdir returns error when it should not
Summary: [udf] [patch] udf fs: readdir returns error when it should not
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 5.3-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Scott Long
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-03-18 15:50 UTC by Andriy Gapon
Modified: 2009-03-23 13:35 UTC (History)
0 users

See Also:


Attachments
bigdir.patch (466 bytes, patch)
2005-03-18 15:50 UTC, Andriy Gapon
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andriy Gapon 2005-03-18 15:50:02 UTC
while reading large enough directory from a udf filesystem,
when total size of directory entries is greater than size of
a userland buffer, udf readdir leaks to userland its internal
error code used to mark such condition. This happens because
the calling code neglects to distinguish between real uiomove()
errors and internal flag set in udf_uiodir() when 
uio->uio_resid < de_size
In my case the error ocurred when a directory had 436 entries in it.
Kernel produced the following messages:
uiomove returned -1

Fix: The following code is based on the similar code in isofs/cd9660, it
keeps proper handling for error==-1, but does not let it be returned
to userland and removes a message about it.

	
How-To-Repeat: 1a. find a UDF disk with a director(y|ies) with many files
or
1b. create a UDF filesystem with a directory with a lot of files in it,
using sysutils/udfclient for example
2. perform ls -l on the directory
3. see that ls returns only a small subset of entries and the following
messages are produced by kernel:
uiomove returned -1
Comment 1 Pawel Jakub Dawidek freebsd_committer freebsd_triage 2005-03-28 10:21:43 UTC
Responsible Changed
From-To: freebsd-bugs->scottl

Assign to author.
Comment 2 Andriy Gapon 2006-05-24 13:49:57 UTC
This problem is still present in 6.1.
The same patch is applicable.

-- 
Andriy Gapon
Comment 3 Robert Jenssen 2009-01-19 12:24:33 UTC
Hi,

To echo Andriy:

This problem is still present in 7.1.
The same patch is applicable. 

Unfortunately, Andriy's patch doesn't seem to be a complete solution. I 
happened to make a DL-DVD having a directory with 640 JPGs. Without the patch 
I get the error message "kernel: uiomove returned -1" when I ls that 
directory. With Andriy's patch "ls" sees the first 67 files. 
Interestingly, "ls -al" reports the first 23 files as expected but then for 
files 24 to 67 says "Input/output error". I had no problems reading this DVD 
on WinXP or Centos5-2. I'd be happy to look further.

Rob Jenssen
Comment 4 Andriy Gapon 2009-01-19 12:40:25 UTC
on 19/01/2009 14:24 Robert Jenssen said the following:
> Hi,
> 
> To echo Andriy:
> 
> This problem is still present in 7.1.
> The same patch is applicable. 
> Unfortunately, Andriy's patch doesn't seem to be a complete solution. I 
> happened to make a DL-DVD having a directory with 640 JPGs. Without the patch 
> I get the error message "kernel: uiomove returned -1" when I ls that 
> directory. With Andriy's patch "ls" sees the first 67 files. 
> Interestingly, "ls -al" reports the first 23 files as expected but then for 
> files 24 to 67 says "Input/output error". I had no problems reading this DVD 
> on WinXP or Centos5-2. I'd be happy to look further.

Please try the patch in:
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/120989

It has more fixes plus enhancements for features and performance.

-- 
Andriy Gapon
Comment 5 Robert Jenssen 2009-01-19 23:35:53 UTC
On Mon, 19 Jan 2009 11:40:25 pm Andriy Gapon wrote:
> on 19/01/2009 14:24 Robert Jenssen said the following:
> > Hi,
> > 
> > To echo Andriy:
> > 
> > This problem is still present in 7.1.
> > The same patch is applicable. 
> > Unfortunately, Andriy's patch doesn't seem to be a complete solution. I 
> > happened to make a DL-DVD having a directory with 640 JPGs. Without the 
patch 
> > I get the error message "kernel: uiomove returned -1" when I ls that 
> > directory. With Andriy's patch "ls" sees the first 67 files. 
> > Interestingly, "ls -al" reports the first 23 files as expected but then 
for 
> > files 24 to 67 says "Input/output error". I had no problems reading this 
DVD 
> > on WinXP or Centos5-2. I'd be happy to look further.
> 
> Please try the patch in:
> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/120989
> 
> It has more fixes plus enhancements for features and performance.
> 
> -- 
> Andriy Gapon
> 

Thanks Andriy. I applied the patch and can now see the entire directory.

Rob Jenssen
Comment 6 dfilter service freebsd_committer freebsd_triage 2009-02-19 15:05:42 UTC
Author: avg
Date: Thu Feb 19 15:05:30 2009
New Revision: 188815
URL: http://svn.freebsd.org/changeset/base/188815

Log:
  fs/udf: fix incorrect error return (-1) when reading a large dir
  
  Not enough space in user-land buffer is not an error, userland
  will read further until eof is reached. So instead of propagating
  -1 to caller we convert it to zero/success.
  
  cd9660 code works exactly the same way.
  
  PR:		kern/78987
  Reviewed by:	jhb (mentor)
  Approved by:	jhb (mentor)

Modified:
  head/sys/fs/udf/udf_vnops.c

Modified: head/sys/fs/udf/udf_vnops.c
==============================================================================
--- head/sys/fs/udf/udf_vnops.c	Thu Feb 19 14:39:52 2009	(r188814)
+++ head/sys/fs/udf/udf_vnops.c	Thu Feb 19 15:05:30 2009	(r188815)
@@ -831,17 +831,16 @@ udf_readdir(struct vop_readdir_args *a)
 			error = udf_uiodir(&uiodir, dir.d_reclen, uio,
 			    ds->this_off);
 		}
-		if (error) {
-			printf("uiomove returned %d\n", error);
+		if (error)
 			break;
-		}
-
 	}
 
 	/* tell the calling layer whether we need to be called again */
 	*a->a_eofflag = uiodir.eofflag;
 	uio->uio_offset = ds->offset + ds->off;
 
+	if(error < 0)
+		error = 0;
 	if (!error)
 		error = ds->error;
 
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 Gavin Atkinson freebsd_committer freebsd_triage 2009-03-23 13:34:42 UTC
State Changed
From-To: open->closed

This has been fixed in HEAD and merged to 7.x (confirmed by avg on IRC)