Bug 164445 - [libc] lseek(2) always returns ENXIO with SEEK_DATA/SEEK_HOLE on 9.0 64bit ZFS
Summary: [libc] lseek(2) always returns ENXIO with SEEK_DATA/SEEK_HOLE on 9.0 64bit ZFS
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Andriy Gapon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-24 15:00 UTC by lge
Modified: 2021-07-15 02:05 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description lge 2012-01-24 15:00:21 UTC
Trying to find holes on a sparse file on 9.0-RELEASE x86_64 ZFS, lseek() with whence=SEEK_DATA or SEEK_HOLE always returns ENXIO. In particular, this code:

offset=lseek(fd,0,SEEK_DATA);
if (offset==-1) {
  if (errno==ENXIO) {
     // No more data
     printf("no more data\n");
     close(fd);
     exit(-1);
  }
}

always prints "no more data". Same thing if seeking with SEEK_HOLE.

The expected behavior is for lseek in this case to return 0, because the first block of data starts at 0.
This works fine on 8.2-RELEASE i386 ZFS.

How-To-Repeat: This code works as expected (offset equals 0 after the call to lseek()) on 8.2-RELEASE i386, but not on 9.0-RELEASE x86_64

offset=lseek(fd,0,SEEK_DATA);
if (offset==-1) {
  if (errno==ENXIO) {
     // No more data
     printf("no more data\n");
     close(fd);
     exit(-1);
  }
}
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2012-01-24 17:02:52 UTC
Responsible Changed
From-To: freebsd-amd64->freebsd-bugs

reclassify.
Comment 2 Andriy Gapon freebsd_committer freebsd_triage 2012-03-07 20:54:48 UTC
I think that you hit the nail on the head.
On FreeBSD the ioctl(2) system call does copyin/copyout of the data argument and
thus those extra copyin/copuout calls in  zfs_ioctl are harmful.

-- 
Andriy Gapon
Comment 3 Mark Linimon 2012-03-12 00:36:41 UTC
----- Forwarded message from Luis Garces-Erice <luis.garces@gmail.com> -----

Date: Wed, 7 Mar 2012 20:53:35 +0100
From: Luis Garces-Erice <luis.garces@gmail.com>
To: freebsd-bugs@freebsd.org
Subject: kern/164445: [zfs][patch] WAS: lseek(2) always returns ENXIO with
	SEEK_DATA/SEEK_HOLE on 9.0 64bit ZFS

Hi all

after digging a bit more into this
(http://www.freebsd.org/cgi/query-pr.cgi?pr=164445), I've found the
problem to be in ZFS or below. The patch attached addresses the
symptom, but the problem remains.

When invoking SEEK_DATA/SEEK_HOLE on a file on ZFS in FreeBSD 9.0 64
bit, the functions ddi_copyin and ddi_copyout in zfs_ioctl() do not
copy the offset passed from the application to the ioctl. The offset
is passed correctly to zfs_ioctl(), though, but those functions copy
garbage into the offset used by zfs_holey(). The corrupted offset is
often bigger than the file, and thus the ioctl returns ENXIO.

The patch does the copy of the offset passed from the application
correctly, and allows lseek(2) with SEEK_DATA/SEEK_HOLE to be used on
ZFS, but it is not a solution. I couldn't see a problem in the
assembler of the copyin and copyout functions in
sys/amd64/amd64/support.S, but I might be wrong, I'm no assembler
expert.


-- 
Luis
****

diff -w -u -r sys.orig/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
--- sys.orig/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c	2012-01-03 04:27:03.000000000 +0100
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c	2012-03-06 11:26:27.000000000 +0100
@@ -296,6 +296,8 @@
 		if (ddi_copyin((void *)data, &off, sizeof (off), flag))
 			return (EFAULT);
 
+		// ddi_copyin did not copy the offset
+		off = (offset_t)*((offset_t *)data);
 		zp = VTOZ(vp);
 		zfsvfs = zp->z_zfsvfs;
 		ZFS_ENTER(zfsvfs);
@@ -308,6 +310,8 @@
 			return (error);
 		if (ddi_copyout(&off, (void *)data, sizeof (off), flag))
 			return (EFAULT);
+		// ddi_copyout did not copy the offset
+		*((offset_t *)data)=off;
 		return (0);
 	}
 	return (ENOTTY);

----- End forwarded message -----
Comment 4 Andriy Gapon freebsd_committer freebsd_triage 2012-04-05 09:00:28 UTC
State Changed
From-To: open->patched

The patch is committed to head. 


Comment 5 Andriy Gapon freebsd_committer freebsd_triage 2012-04-05 09:00:28 UTC
Responsible Changed
From-To: freebsd-bugs->avg

I am handling this one.
Comment 6 dfilter service freebsd_committer freebsd_triage 2012-04-05 09:00:40 UTC
Author: avg
Date: Thu Apr  5 07:59:59 2012
New Revision: 233918
URL: http://svn.freebsd.org/changeset/base/233918

Log:
  zfs_ioctl: no need for ddi_copyin/out here because sys_ioctl handles that
  
  On FreeBSD the direct ioctl argument is automatically copied in/out
  as necesary by the kernel ioctl entry point.
  
  PR:		kern/164445
  Submitted by:	Luis Garces-Erice <lge@ieee.org>
  Tested by:	Attila Nagy <bra@fsn.hu>
  MFC after:	5 days

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c	Thu Apr  5 04:41:06 2012	(r233917)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c	Thu Apr  5 07:59:59 2012	(r233918)
@@ -293,9 +293,12 @@ zfs_ioctl(vnode_t *vp, u_long com, intpt
 
 	case _FIO_SEEK_DATA:
 	case _FIO_SEEK_HOLE:
+#ifdef sun
 		if (ddi_copyin((void *)data, &off, sizeof (off), flag))
 			return (EFAULT);
-
+#else
+		off = *(offset_t *)data;
+#endif
 		zp = VTOZ(vp);
 		zfsvfs = zp->z_zfsvfs;
 		ZFS_ENTER(zfsvfs);
@@ -306,8 +309,12 @@ zfs_ioctl(vnode_t *vp, u_long com, intpt
 		ZFS_EXIT(zfsvfs);
 		if (error)
 			return (error);
+#ifdef sun
 		if (ddi_copyout(&off, (void *)data, sizeof (off), flag))
 			return (EFAULT);
+#else
+		*(offset_t *)data = off;
+#endif
 		return (0);
 	}
 	return (ENOTTY);
_______________________________________________
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 Andriy Gapon freebsd_committer freebsd_triage 2012-04-16 13:09:42 UTC
State Changed
From-To: patched->closed

The fix is put into all relevant supported branches.