Re: CFR: lseek() POSIXed patch
Updated variant:
--- vfs_syscalls.c.old Sat Aug 11 02:14:18 2001
+++ vfs_syscalls.c Sun Aug 19 05:01:32 2001
@@ -1614,29 +1614,44 @@
register struct filedesc *fdp = p-p_fd;
register struct file *fp;
struct vattr vattr;
- int error;
+ struct vnode *vp;
+ off_t offset;
+ int error, noneg;
if ((u_int)SCARG(uap, fd) = fdp-fd_nfiles ||
(fp = fdp-fd_ofiles[SCARG(uap, fd)]) == NULL)
return (EBADF);
if (fp-f_type != DTYPE_VNODE)
return (ESPIPE);
+ vp = (struct vnode *)fp-f_data;
+ noneg = (vp-v_type != VCHR);
+ offset = SCARG(uap, offset);
switch (SCARG(uap, whence)) {
case L_INCR:
- fp-f_offset += SCARG(uap, offset);
+ if (noneg
+ ((offset 0 fp-f_offset OFF_MAX - offset) ||
+(offset 0 fp-f_offset OFF_MIN - offset)))
+ return (EOVERFLOW);
+ offset += fp-f_offset;
break;
case L_XTND:
- error=VOP_GETATTR((struct vnode *)fp-f_data, vattr, cred, p);
+ error = VOP_GETATTR(vp, vattr, cred, p);
if (error)
return (error);
- fp-f_offset = SCARG(uap, offset) + vattr.va_size;
+ if (noneg
+ ((offset 0 vattr.va_size OFF_MAX - offset) ||
+(offset 0 vattr.va_size OFF_MIN - offset)))
+ return (EOVERFLOW);
+ offset += vattr.va_size;
break;
case L_SET:
- fp-f_offset = SCARG(uap, offset);
break;
default:
return (EINVAL);
}
+ if (noneg offset 0)
+ return (EINVAL);
+ fp-f_offset = offset;
*(off_t *)(p-p_retval) = fp-f_offset;
return (0);
}
--
Andrey A. Chernov
http://ache.pp.ru/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
CFR: lseek() POSIXed patch
Here it is what POSIX says about lseek():
[EINVAL]
The whence argument is not a proper value, or the resulting file offset
would be negative for a regular file, block special file, or directory.
[EOVERFLOW]
The resulting file offset would be a value which cannot be represented
correctly in an object of type off_t.
The patch below adds both cases, i.e. disallow negative seeks for VREG,
VDIR, VBLK and add off_t overflow checks.
I plan to commit this, please review.
--- vfs_syscalls.c.old Wed Aug 15 04:45:30 2001
+++ vfs_syscalls.c Wed Aug 15 12:46:12 2001
@@ -1614,29 +1614,44 @@
register struct filedesc *fdp = p-p_fd;
register struct file *fp;
struct vattr vattr;
- int error;
+ off_t offset;
+ int error, no_neg_seek;
if ((u_int)SCARG(uap, fd) = fdp-fd_nfiles ||
(fp = fdp-fd_ofiles[SCARG(uap, fd)]) == NULL)
return (EBADF);
if (fp-f_type != DTYPE_VNODE)
return (ESPIPE);
+ error = VOP_GETATTR((struct vnode *)fp-f_data, vattr, cred, p);
+ no_neg_seek = (!error
+ (vattr.va_type == VREG ||
+ vattr.va_type == VDIR ||
+ vattr.va_type == VBLK));
+ offset = SCARG(uap, offset);
switch (SCARG(uap, whence)) {
case L_INCR:
- fp-f_offset += SCARG(uap, offset);
+ if ((fp-f_offset 0 offset 0
+offset + fp-f_offset 0) ||
+ (fp-f_offset 0 offset 0
+offset + fp-f_offset 0))
+ return (EOVERFLOW);
+ offset += fp-f_offset;
break;
case L_XTND:
- error=VOP_GETATTR((struct vnode *)fp-f_data, vattr, cred, p);
if (error)
return (error);
- fp-f_offset = SCARG(uap, offset) + vattr.va_size;
+ if (offset 0 (off_t)(offset + vattr.va_size) 0)
+ return (EOVERFLOW);
+ offset += vattr.va_size;
break;
case L_SET:
- fp-f_offset = SCARG(uap, offset);
break;
default:
return (EINVAL);
}
+ if (no_neg_seek offset 0)
+ return (EINVAL);
+ fp-f_offset = offset;
*(off_t *)(p-p_retval) = fp-f_offset;
return (0);
}
--
Andrey A. Chernov
http://ache.pp.ru/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: CFR: lseek() POSIXed patch
On Wed, 15 Aug 2001, Andrey A. Chernov wrote:
The patch below adds both cases, i.e. disallow negative seeks for VREG,
VDIR, VBLK and add off_t overflow checks.
I plan to commit this, please review.
--- vfs_syscalls.c.oldWed Aug 15 04:45:30 2001
+++ vfs_syscalls.cWed Aug 15 12:46:12 2001
@@ -1614,29 +1614,44 @@
register struct filedesc *fdp = p-p_fd;
register struct file *fp;
struct vattr vattr;
- int error;
+ off_t offset;
+ int error, no_neg_seek;
if ((u_int)SCARG(uap, fd) = fdp-fd_nfiles ||
(fp = fdp-fd_ofiles[SCARG(uap, fd)]) == NULL)
return (EBADF);
if (fp-f_type != DTYPE_VNODE)
return (ESPIPE);
+ error = VOP_GETATTR((struct vnode *)fp-f_data, vattr, cred, p);
+ no_neg_seek = (!error
+(vattr.va_type == VREG ||
+ vattr.va_type == VDIR ||
+ vattr.va_type == VBLK));
The type is in vp-v_type (where vp = (struct vnode *)fp-f_data), so
the VOP_GETTATTR() call can be avoided except in the L_XTND case, as
in the old code.
I think permitting negative offsets for the VCHR case only is enough.
+ offset = SCARG(uap, offset);
switch (SCARG(uap, whence)) {
case L_INCR:
- fp-f_offset += SCARG(uap, offset);
+ if ((fp-f_offset 0 offset 0
+ offset + fp-f_offset 0) ||
+ (fp-f_offset 0 offset 0
+ offset + fp-f_offset 0))
+ return (EOVERFLOW);
You used a slightly simpler, slightly less ugly overflow checks in
fseek.c. This seems to be a bug in fseek.c. Since the checks are so
messy, maybe they should be messy enough to not depend on undefined
behaviour (they check for overflow after overflow may have occurred).
Something like:
#define OFF_T_MAX 0x7FFF /* XXX */
#define OFF_T_MIN (-0x7FFF - 1) /* XXX */
start = fp-f_offset;
if (offset 0 start OFF_T_MAX - offset ||
offset 0 start OFF_T_MIN - offset)
return (EOVERFLOW);
+ if ((fp-f_offset 0 offset 0
+ offset + fp-f_offset 0) ||
+ (fp-f_offset 0 offset 0
+ offset + fp-f_offset 0))
+ offset += fp-f_offset;
break;
case L_XTND:
- error=VOP_GETATTR((struct vnode *)fp-f_data, vattr, cred, p);
if (error)
return (error);
- fp-f_offset = SCARG(uap, offset) + vattr.va_size;
+ if (offset 0 (off_t)(offset + vattr.va_size) 0)
+ return (EOVERFLOW);
+ offset += vattr.va_size;
This assumes that vattr.va_size is representable as a (non-negative) off_t.
E.g., if offset is 1 and vattr.va_size is (uquad_t)-1, then the sum overflows
but the overflow is not detected.
break;
case L_SET:
- fp-f_offset = SCARG(uap, offset);
break;
default:
return (EINVAL);
}
+ if (no_neg_seek offset 0)
+ return (EINVAL);
Some no_neg_seek conditionals are needed for the overflow checks too.
Otherwise seeking from offset OFF_T_MAX to 1 more than that is disallowed
for all types of files. The offset overflows, but it only reaches the
half way mark in a 64-bit /dev/kmem.
+ fp-f_offset = offset;
*(off_t *)(p-p_retval) = fp-f_offset;
return (0);
}
kern_lockf.c has some related problems. It doesn't distinguish the
EOVERFLOW case from the EINVAL case as is now required by POSIX. POSIX
now specifies the behaviour for weird cases like negative lengths (it
gives an interval extending from the top down), but I made the overflow
checking too strong to permit this.
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: CFR: lseek() POSIXed patch
On Wed, Aug 15, 2001 at 20:15:21 +1000, Bruce Evans wrote: Something like: #define OFF_T_MAX 0x7FFF /* XXX */ #define OFF_T_MIN (-0x7FFF - 1) /* XXX */ It seems that this defines often needed in many places. What about adding them to /usr/include/machine/limits.h ? Is 0x7fffLL form will be better (i.e. long long)? For other things you mention: I'll try to resolve them. -- Andrey A. Chernov http://ache.pp.ru/ To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
