CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: riastradh
Date: Wed Feb 22 21:49:45 UTC 2023
Modified Files:
src/sys/ufs/ufs: quota1_subr.c quota2.h quota2_subr.c ufs_acl.c
ufs_extattr.c ufs_lookup.c ufs_quota.c ufs_quota1.c ufs_quota2.c
ufs_vfsops.c
Log Message:
ufs: Nix trailing whitespace and tidy up some other minor KNF.
To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/sys/ufs/ufs/quota1_subr.c
cvs rdiff -u -r1.10 -r1.11 src/sys/ufs/ufs/quota2.h
cvs rdiff -u -r1.5 -r1.6 src/sys/ufs/ufs/quota2_subr.c
cvs rdiff -u -r1.4 -r1.5 src/sys/ufs/ufs/ufs_acl.c
cvs rdiff -u -r1.53 -r1.54 src/sys/ufs/ufs/ufs_extattr.c
cvs rdiff -u -r1.156 -r1.157 src/sys/ufs/ufs/ufs_lookup.c
cvs rdiff -u -r1.117 -r1.118 src/sys/ufs/ufs/ufs_quota.c
cvs rdiff -u -r1.25 -r1.26 src/sys/ufs/ufs/ufs_quota1.c
cvs rdiff -u -r1.45 -r1.46 src/sys/ufs/ufs/ufs_quota2.c
cvs rdiff -u -r1.60 -r1.61 src/sys/ufs/ufs/ufs_vfsops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/quota1_subr.c
diff -u src/sys/ufs/ufs/quota1_subr.c:1.7 src/sys/ufs/ufs/quota1_subr.c:1.8
--- src/sys/ufs/ufs/quota1_subr.c:1.7 Sun Jan 29 06:23:20 2012
+++ src/sys/ufs/ufs/quota1_subr.c Wed Feb 22 21:49:45 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: quota1_subr.c,v 1.7 2012/01/29 06:23:20 dholland Exp $ */
+/* $NetBSD: quota1_subr.c,v 1.8 2023/02/22 21:49:45 riastradh Exp $ */
/*-
* Copyright (c) 2010 Manuel Bouyer
* All rights reserved.
@@ -26,7 +26,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: quota1_subr.c,v 1.7 2012/01/29 06:23:20 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: quota1_subr.c,v 1.8 2023/02/22 21:49:45 riastradh Exp $");
#include
#include
@@ -54,7 +54,7 @@ q2e2dqblk_limit(uint64_t lim)
void
dqblk_to_quotavals(const struct dqblk *dqblk,
- struct quotaval *blocks, struct quotaval *files)
+struct quotaval *blocks, struct quotaval *files)
{
/* XXX is qv_grace getting handled correctly? */
@@ -71,7 +71,7 @@ dqblk_to_quotavals(const struct dqblk *d
void
quotavals_to_dqblk(const struct quotaval *blocks, const struct quotaval *files,
- struct dqblk *dqblk)
+struct dqblk *dqblk)
{
/* XXX is qv_grace getting handled correctly? */
@@ -85,4 +85,3 @@ quotavals_to_dqblk(const struct quotaval
dqblk->dqb_curinodes = files->qv_usage;
dqblk->dqb_itime = files->qv_expiretime;
}
-
Index: src/sys/ufs/ufs/quota2.h
diff -u src/sys/ufs/ufs/quota2.h:1.10 src/sys/ufs/ufs/quota2.h:1.11
--- src/sys/ufs/ufs/quota2.h:1.10 Wed Oct 25 18:06:01 2017
+++ src/sys/ufs/ufs/quota2.h Wed Feb 22 21:49:45 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: quota2.h,v 1.10 2017/10/25 18:06:01 jdolecek Exp $ */
+/* $NetBSD: quota2.h,v 1.11 2023/02/22 21:49:45 riastradh Exp $ */
/*-
* Copyright (c) 2010 Manuel Bouyer
* All rights reserved.
@@ -115,7 +115,7 @@ void quota2_ufs_rwq2e(const struct quota
#define QL_S_ALLOW_SOFT 0x01 /* over soft limit */
#define QL_S_DENY_GRACE 0x02 /* over soft limit, grace time expired */
#define QL_S_DENY_HARD 0x03 /* over hard limit */
-
+
#define QL_F_CROSS 0x80 /* crossing soft limit */
#define QL_STATUS(x) ((x) & 0x0f)
Index: src/sys/ufs/ufs/quota2_subr.c
diff -u src/sys/ufs/ufs/quota2_subr.c:1.5 src/sys/ufs/ufs/quota2_subr.c:1.6
--- src/sys/ufs/ufs/quota2_subr.c:1.5 Sun Feb 5 14:19:04 2012
+++ src/sys/ufs/ufs/quota2_subr.c Wed Feb 22 21:49:45 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: quota2_subr.c,v 1.5 2012/02/05 14:19:04 dholland Exp $ */
+/* $NetBSD: quota2_subr.c,v 1.6 2023/02/22 21:49:45 riastradh Exp $ */
/*-
* Copyright (c) 2010, 2011 Manuel Bouyer
* All rights reserved.
@@ -26,7 +26,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: quota2_subr.c,v 1.5 2012/02/05 14:19:04 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: quota2_subr.c,v 1.6 2023/02/22 21:49:45 riastradh Exp $");
#include
#include
@@ -87,7 +87,8 @@ quota2_create_blk0(uint64_t bsize, void
}
void
-quota2_ufs_rwq2v(const struct quota2_val *s, struct quota2_val *d, int needswap)
+quota2_ufs_rwq2v(const struct quota2_val *s, struct quota2_val *d,
+int needswap)
{
d->q2v_hardlimit = ufs_rw64(s->q2v_hardlimit, needswap);
d->q2v_softlimit = ufs_rw64(s->q2v_softlimit, needswap);
@@ -98,7 +99,7 @@ quota2_ufs_rwq2v(const struct quota2_val
void
quota2_ufs_rwq2e(const struct quota2_entry *s, struct quota2_entry *d,
-int needswap)
+int needswap)
{
quota2_ufs_rwq2v(>q2e_val[QL_BLOCK], >q2e_val[QL_BLOCK],
needswap);
@@ -110,7 +111,7 @@ int needswap)
int
quota_check_limit(uint64_t cur, uint64_t change, uint64_t soft, uint64_t hard,
time_t expire, time_t now)
-{
+{
if (cur + change > hard) {
if (cur <= soft)
return (QL_F_CROSS | QL_S_DENY_HARD);
@@ -124,4 +125,4 @@ quota_check_limit(uint64_t cur, uint64_t
return QL_S_ALLOW_SOFT;
}
return QL_S_ALLOW_OK;
-}
+}
Index: src/sys/ufs/ufs/ufs_acl.c
diff -u src/sys/ufs/ufs/ufs_acl.c:1.4
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: riastradh Date: Wed Feb 22 21:49:45 UTC 2023 Modified Files: src/sys/ufs/ufs: quota1_subr.c quota2.h quota2_subr.c ufs_acl.c ufs_extattr.c ufs_lookup.c ufs_quota.c ufs_quota1.c ufs_quota2.c ufs_vfsops.c Log Message: ufs: Nix trailing whitespace and tidy up some other minor KNF. To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/ufs/ufs/quota1_subr.c cvs rdiff -u -r1.10 -r1.11 src/sys/ufs/ufs/quota2.h cvs rdiff -u -r1.5 -r1.6 src/sys/ufs/ufs/quota2_subr.c cvs rdiff -u -r1.4 -r1.5 src/sys/ufs/ufs/ufs_acl.c cvs rdiff -u -r1.53 -r1.54 src/sys/ufs/ufs/ufs_extattr.c cvs rdiff -u -r1.156 -r1.157 src/sys/ufs/ufs/ufs_lookup.c cvs rdiff -u -r1.117 -r1.118 src/sys/ufs/ufs/ufs_quota.c cvs rdiff -u -r1.25 -r1.26 src/sys/ufs/ufs/ufs_quota1.c cvs rdiff -u -r1.45 -r1.46 src/sys/ufs/ufs/ufs_quota2.c cvs rdiff -u -r1.60 -r1.61 src/sys/ufs/ufs/ufs_vfsops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: andvar Date: Mon Jan 30 13:45:26 UTC 2023 Modified Files: src/sys/ufs/ufs: README.acls Log Message: s/isses/issues/ To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/sys/ufs/ufs/README.acls Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/ufs/ufs/README.acls diff -u src/sys/ufs/ufs/README.acls:1.1 src/sys/ufs/ufs/README.acls:1.2 --- src/sys/ufs/ufs/README.acls:1.1 Sat May 16 18:31:54 2020 +++ src/sys/ufs/ufs/README.acls Mon Jan 30 13:45:26 2023 @@ -45,7 +45,7 @@ will result in incorrect application of the kernel is not configured for ACL support, a warning will be printed by the kernel at mount-time. For reliability purposes, it is recommended that the superblock flag be used instead of the -mount-time flag, as this will avoid re-mount isses with the root file +mount-time flag, as this will avoid re-mount issues with the root file system. For reliability and performance reasons, the use of ACLs on UFS1 is discouraged; UFS2 extended attributes provide a more reliable storage mechanism for ACLs.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: andvar Date: Mon Jan 30 13:45:26 UTC 2023 Modified Files: src/sys/ufs/ufs: README.acls Log Message: s/isses/issues/ To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/sys/ufs/ufs/README.acls Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: hannken Date: Tue Apr 26 15:37:25 UTC 2022 Modified Files: src/sys/ufs/ufs: ufs_quota1.c Log Message: Keep flag "UFS_QUOTA" set until the last quota is closed. Prevents a live lock when dqrele() finds a struct with "dq_cnt == 1" and flag "DQ_MOD" and cannot sync as flag UFS_QUOTA is unset. To generate a diff of this commit: cvs rdiff -u -r1.24 -r1.25 src/sys/ufs/ufs/ufs_quota1.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/ufs/ufs/ufs_quota1.c diff -u src/sys/ufs/ufs/ufs_quota1.c:1.24 src/sys/ufs/ufs/ufs_quota1.c:1.25 --- src/sys/ufs/ufs/ufs_quota1.c:1.24 Tue Jun 29 22:40:54 2021 +++ src/sys/ufs/ufs/ufs_quota1.c Tue Apr 26 15:37:25 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_quota1.c,v 1.24 2021/06/29 22:40:54 dholland Exp $ */ +/* $NetBSD: ufs_quota1.c,v 1.25 2022/04/26 15:37:25 hannken Exp $ */ /* * Copyright (c) 1982, 1986, 1990, 1993, 1995 @@ -35,7 +35,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ufs_quota1.c,v 1.24 2021/06/29 22:40:54 dholland Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ufs_quota1.c,v 1.25 2022/04/26 15:37:25 hannken Exp $"); #include #include @@ -433,7 +433,6 @@ quota1_handle_cmd_quotaoff(struct lwp *l return (0); } ump->umq1_qflags[type] |= QTF_CLOSING; - ump->um_flags &= ~UFS_QUOTA; mutex_exit(); /* * Search vnodes associated with this mount point, @@ -470,6 +469,8 @@ quota1_handle_cmd_quotaoff(struct lwp *l if (ump->um_quotas[i] != NULLVP) break; ump->umq1_qflags[type] &= ~QTF_CLOSING; + if (i == MAXQUOTAS) + ump->um_flags &= ~UFS_QUOTA; cv_broadcast(); mutex_exit(); kauth_cred_free(cred);
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: hannken Date: Tue Apr 26 15:37:25 UTC 2022 Modified Files: src/sys/ufs/ufs: ufs_quota1.c Log Message: Keep flag "UFS_QUOTA" set until the last quota is closed. Prevents a live lock when dqrele() finds a struct with "dq_cnt == 1" and flag "DQ_MOD" and cannot sync as flag UFS_QUOTA is unset. To generate a diff of this commit: cvs rdiff -u -r1.24 -r1.25 src/sys/ufs/ufs/ufs_quota1.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: hannken Date: Mon Feb 21 17:07:45 UTC 2022 Modified Files: src/sys/ufs/ufs: ufs_readwrite.c Log Message: Fix wrong assertion, the negatiopn of "a && b" is "!a || !b" so we need "DIP(ip, blocks) != 0" here. Should fix PR kern/56725 (Panic when ls directory with device nodes on an older ffs) To generate a diff of this commit: cvs rdiff -u -r1.127 -r1.128 src/sys/ufs/ufs/ufs_readwrite.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/ufs/ufs/ufs_readwrite.c diff -u src/sys/ufs/ufs/ufs_readwrite.c:1.127 src/sys/ufs/ufs/ufs_readwrite.c:1.128 --- src/sys/ufs/ufs/ufs_readwrite.c:1.127 Wed Oct 20 03:08:19 2021 +++ src/sys/ufs/ufs/ufs_readwrite.c Mon Feb 21 17:07:45 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_readwrite.c,v 1.127 2021/10/20 03:08:19 thorpej Exp $ */ +/* $NetBSD: ufs_readwrite.c,v 1.128 2022/02/21 17:07:45 hannken Exp $ */ /*- * Copyright (c) 1993 @@ -32,7 +32,7 @@ */ #include -__KERNEL_RCSID(1, "$NetBSD: ufs_readwrite.c,v 1.127 2021/10/20 03:08:19 thorpej Exp $"); +__KERNEL_RCSID(1, "$NetBSD: ufs_readwrite.c,v 1.128 2022/02/21 17:07:45 hannken Exp $"); #define FS struct fs #define I_FS i_fs @@ -142,7 +142,7 @@ BUFRD(struct vnode *vp, struct uio *uio, KASSERT(vp->v_type != VLNK || ip->i_size >= ump->um_maxsymlinklen); KASSERT(vp->v_type != VLNK || ump->um_maxsymlinklen != 0 || - DIP(ip, blocks) == 0); + DIP(ip, blocks) != 0); if (uio->uio_offset > ump->um_maxfilesize) return EFBIG;
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: hannken Date: Mon Feb 21 17:07:45 UTC 2022 Modified Files: src/sys/ufs/ufs: ufs_readwrite.c Log Message: Fix wrong assertion, the negatiopn of "a && b" is "!a || !b" so we need "DIP(ip, blocks) != 0" here. Should fix PR kern/56725 (Panic when ls directory with device nodes on an older ffs) To generate a diff of this commit: cvs rdiff -u -r1.127 -r1.128 src/sys/ufs/ufs/ufs_readwrite.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: christos
Date: Fri Nov 26 17:35:12 UTC 2021
Modified Files:
src/sys/ufs/ufs: ufs_acl.c ufs_vnops.c
Log Message:
use MNT_NFS4ACLS instead of MNT_ACLS (which was changed before to mean
MNT_POSIX1EACLS)
To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/sys/ufs/ufs/ufs_acl.c
cvs rdiff -u -r1.260 -r1.261 src/sys/ufs/ufs/ufs_vnops.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/ufs_acl.c
diff -u src/sys/ufs/ufs/ufs_acl.c:1.3 src/sys/ufs/ufs/ufs_acl.c:1.4
--- src/sys/ufs/ufs/ufs_acl.c:1.3 Tue Oct 19 23:08:19 2021
+++ src/sys/ufs/ufs/ufs_acl.c Fri Nov 26 12:35:12 2021
@@ -36,7 +36,7 @@
#if 0
__FBSDID("$FreeBSD: head/sys/ufs/ufs/ufs_acl.c 356669 2020-01-13 02:31:51Z mjg $");
#endif
-__KERNEL_RCSID(0, "$NetBSD: ufs_acl.c,v 1.3 2021/10/20 03:08:19 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_acl.c,v 1.4 2021/11/26 17:35:12 christos Exp $");
#if defined(_KERNEL_OPT)
#include "opt_ffs.h"
@@ -215,7 +215,7 @@ ufs_getacl_nfs4(struct vop_getacl_args *
{
int error;
- if ((ap->a_vp->v_mount->mnt_flag & MNT_ACLS) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
return (EINVAL);
error = VOP_ACCESSX(ap->a_vp, VREAD_ACL, ap->a_cred);
@@ -362,7 +362,7 @@ ufs_getacl(void *v)
{
struct vop_getacl_args *ap = v;
- if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_ACLS)) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_NFS4ACLS)) == 0)
return (EOPNOTSUPP);
if (ap->a_type == ACL_TYPE_NFS4)
@@ -438,7 +438,7 @@ ufs_setacl_nfs4(struct vop_setacl_args *
int error;
struct inode *ip = VTOI(ap->a_vp);
- if ((ap->a_vp->v_mount->mnt_flag & MNT_ACLS) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
return (EINVAL);
if (ap->a_vp->v_mount->mnt_flag & MNT_RDONLY)
@@ -612,7 +612,7 @@ int
ufs_setacl(void *v)
{
struct vop_setacl_args *ap = v;
- if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_ACLS)) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_NFS4ACLS)) == 0)
return (EOPNOTSUPP);
if (ap->a_type == ACL_TYPE_NFS4)
@@ -627,7 +627,7 @@ ufs_aclcheck_nfs4(struct vop_aclcheck_ar
{
int is_directory = 0;
- if ((ap->a_vp->v_mount->mnt_flag & MNT_ACLS) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
return (EINVAL);
/*
@@ -683,7 +683,7 @@ ufs_aclcheck(void *v)
{
struct vop_aclcheck_args *ap = v;
- if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_ACLS)) == 0)
+ if ((ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_NFS4ACLS)) == 0)
return (EOPNOTSUPP);
if (ap->a_type == ACL_TYPE_NFS4)
Index: src/sys/ufs/ufs/ufs_vnops.c
diff -u src/sys/ufs/ufs/ufs_vnops.c:1.260 src/sys/ufs/ufs/ufs_vnops.c:1.261
--- src/sys/ufs/ufs/ufs_vnops.c:1.260 Tue Oct 19 23:08:19 2021
+++ src/sys/ufs/ufs/ufs_vnops.c Fri Nov 26 12:35:12 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_vnops.c,v 1.260 2021/10/20 03:08:19 thorpej Exp $ */
+/* $NetBSD: ufs_vnops.c,v 1.261 2021/11/26 17:35:12 christos Exp $ */
/*-
* Copyright (c) 2008, 2020 The NetBSD Foundation, Inc.
@@ -66,7 +66,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.260 2021/10/20 03:08:19 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.261 2021/11/26 17:35:12 christos Exp $");
#if defined(_KERNEL_OPT)
#include "opt_ffs.h"
@@ -345,8 +345,8 @@ ufs_accessx(void *v)
return error;
#ifdef UFS_ACL
- if ((vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_ACLS)) != 0) {
- if (vp->v_mount->mnt_flag & MNT_ACLS)
+ if ((vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_NFS4ACLS)) != 0) {
+ if (vp->v_mount->mnt_flag & MNT_NFS4ACLS)
type = ACL_TYPE_NFS4;
else
type = ACL_TYPE_ACCESS;
@@ -730,7 +730,7 @@ ufs_chmod(struct vnode *vp, int mode, ka
return (error);
#ifdef UFS_ACL
- if ((vp->v_mount->mnt_flag & MNT_ACLS) != 0) {
+ if ((vp->v_mount->mnt_flag & MNT_NFS4ACLS) != 0) {
error = ufs_update_nfs4_acl_after_mode_change(vp, mode,
ip->i_uid, cred, l);
if (error)
@@ -1291,7 +1291,7 @@ ufs_mkdir(void *v)
cnp->cn_cred, l);
if (error)
goto bad;
- } else if (dvp->v_mount->mnt_flag & MNT_ACLS) {
+ } else if (dvp->v_mount->mnt_flag & MNT_NFS4ACLS) {
error = ufs_do_nfs4_acl_inheritance(dvp, tvp, dmode,
cnp->cn_cred, l);
if (error)
@@ -2105,7 +2105,7 @@ ufs_pathconf(void *v)
*ap->a_retval = 0;
return 0;
case _PC_ACL_NFS4:
- if (ap->a_vp->v_mount->mnt_flag & MNT_ACLS)
+ if (ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS)
*ap->a_retval = 1;
else
*ap->a_retval = 0;
@@ -2113,7 +2113,7 @@ ufs_pathconf(void *v)
#endif
case _PC_ACL_PATH_MAX:
#ifdef UFS_ACL
- if (ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_ACLS))
+ if (ap->a_vp->v_mount->mnt_flag & (MNT_POSIX1EACLS | MNT_NFS4ACLS))
*ap->a_retval = ACL_MAX_ENTRIES;
else
*ap->a_retval = 3;
@@ -2272,7 +2272,7 @@
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: christos Date: Fri Nov 26 17:35:12 UTC 2021 Modified Files: src/sys/ufs/ufs: ufs_acl.c ufs_vnops.c Log Message: use MNT_NFS4ACLS instead of MNT_ACLS (which was changed before to mean MNT_POSIX1EACLS) To generate a diff of this commit: cvs rdiff -u -r1.3 -r1.4 src/sys/ufs/ufs/ufs_acl.c cvs rdiff -u -r1.260 -r1.261 src/sys/ufs/ufs/ufs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: thorpej
Date: Sun Oct 10 23:02:10 UTC 2021
Modified Files:
src/sys/ufs/ufs: ufs_acl.c
Log Message:
Use VN_KNOTE() to send our NOTE_ATTRIB and NOTE_REVOKE events.
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/ufs/ufs/ufs_acl.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/ufs_acl.c
diff -u src/sys/ufs/ufs/ufs_acl.c:1.1 src/sys/ufs/ufs/ufs_acl.c:1.2
--- src/sys/ufs/ufs/ufs_acl.c:1.1 Sat May 16 18:31:54 2020
+++ src/sys/ufs/ufs/ufs_acl.c Sun Oct 10 23:02:10 2021
@@ -36,7 +36,7 @@
#if 0
__FBSDID("$FreeBSD: head/sys/ufs/ufs/ufs_acl.c 356669 2020-01-13 02:31:51Z mjg $");
#endif
-__KERNEL_RCSID(0, "$NetBSD: ufs_acl.c,v 1.1 2020/05/16 18:31:54 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_acl.c,v 1.2 2021/10/10 23:02:10 thorpej Exp $");
#if defined(_KERNEL_OPT)
#include "opt_ffs.h"
@@ -425,7 +425,7 @@ ufs_setacl_nfs4_internal(struct vnode *v
DIP_ASSIGN(ip, mode, ip->i_mode);
ip->i_flag |= IN_CHANGE;
- KNOTE(>v_klist, NOTE_REVOKE);
+ VN_KNOTE(vp, NOTE_REVOKE);
error = UFS_UPDATE(vp, NULL, NULL, 0);
if (lock)
@@ -607,7 +607,7 @@ ufs_setacl_posix1e(struct vnode *vp, int
UFS_WAPBL_END(vp->v_mount);
}
- KNOTE(>v_klist, NOTE_ATTRIB);
+ VN_KNOTE(vp, NOTE_ATTRIB);
return (error);
}
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: thorpej Date: Sun Oct 10 23:02:10 UTC 2021 Modified Files: src/sys/ufs/ufs: ufs_acl.c Log Message: Use VN_KNOTE() to send our NOTE_ATTRIB and NOTE_REVOKE events. To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/sys/ufs/ufs/ufs_acl.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/ufs/ufs
Le 27/02/2020 à 01:36, Simon Burge a écrit : > "Maxime Villard" wrote: > >> Module Name: src >> Committed By:maxv >> Date:Wed Feb 26 18:00:12 UTC 2020 >> >> Modified Files: >> >> src/sys/ufs/ufs: ufs_vnops.c >> >> Log Message: >> >> Zero out the padding in 'd_namlen', to prevent info leaks. Same logic as >> ufs_makedirentry(). > > Is it cleaner to just call pool_cache_get() with PR_ZERO? > > Cheers, > Simon. In this specific case there is already a clean macro that gives the number of padding bytes, so using it is cleaner/faster than zeroing the whole buffer. Maxime
Re: CVS commit: src/sys/ufs/ufs
"Maxime Villard" wrote: > Module Name: src > Committed By: maxv > Date: Wed Feb 26 18:00:12 UTC 2020 > > Modified Files: > > src/sys/ufs/ufs: ufs_vnops.c > > Log Message: > > Zero out the padding in 'd_namlen', to prevent info leaks. Same logic as > ufs_makedirentry(). Is it cleaner to just call pool_cache_get() with PR_ZERO? Cheers, Simon.
CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: christos
Date: Mon Aug 19 14:09:12 UTC 2019
Modified Files:
src/sys/ufs/ufs: ufs_extattr.c
Log Message:
- KNF more
- print the full path in error messages
To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/sys/ufs/ufs/ufs_extattr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/ufs_extattr.c
diff -u src/sys/ufs/ufs/ufs_extattr.c:1.49 src/sys/ufs/ufs/ufs_extattr.c:1.50
--- src/sys/ufs/ufs/ufs_extattr.c:1.49 Mon Aug 19 05:30:30 2019
+++ src/sys/ufs/ufs/ufs_extattr.c Mon Aug 19 10:09:12 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_extattr.c,v 1.49 2019/08/19 09:30:30 christos Exp $ */
+/* $NetBSD: ufs_extattr.c,v 1.50 2019/08/19 14:09:12 christos Exp $ */
/*-
* Copyright (c) 1999-2002 Robert N. M. Watson
@@ -48,7 +48,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.49 2019/08/19 09:30:30 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.50 2019/08/19 14:09:12 christos Exp $");
#ifdef _KERNEL_OPT
#include "opt_ffs.h"
@@ -238,17 +238,13 @@ ufs_extattr_autocreate_attr(struct vnode
switch (attrnamespace) {
case EXTATTR_NAMESPACE_SYSTEM:
(void)snprintf(path, PATH_MAX, "%s/%s/%s/%s",
- mp->mnt_stat.f_mntonname,
- UFS_EXTATTR_FSROOTSUBDIR,
- UFS_EXTATTR_SUBDIR_SYSTEM,
- attrname);
+ mp->mnt_stat.f_mntonname, UFS_EXTATTR_FSROOTSUBDIR,
+ UFS_EXTATTR_SUBDIR_SYSTEM, attrname);
break;
case EXTATTR_NAMESPACE_USER:
(void)snprintf(path, PATH_MAX, "%s/%s/%s/%s",
- mp->mnt_stat.f_mntonname,
- UFS_EXTATTR_FSROOTSUBDIR,
- UFS_EXTATTR_SUBDIR_USER,
- attrname);
+ mp->mnt_stat.f_mntonname, UFS_EXTATTR_FSROOTSUBDIR,
+ UFS_EXTATTR_SUBDIR_USER, attrname);
break;
default:
PNBUF_PUT(path);
@@ -666,16 +662,18 @@ ufs_extattr_subdir(struct lwp *l, struct
error = ufs_extattr_lookup(attr_dvp, LOCKPARENT, subdir, _sub, l);
KASSERT(VOP_ISLOCKED(attr_dvp) == LK_EXCLUSIVE);
if (error) {
- printf("%s: Can't find `%s/%s' (%d)\n",
- __func__, UFS_EXTATTR_FSROOTSUBDIR, subdir, error);
+ printf("%s: Can't find `%s/%s/%s' (%d)\n",
+ __func__, mp->mnt_stat.f_mntonname,
+ UFS_EXTATTR_FSROOTSUBDIR, subdir, error);
return error;
}
KASSERT(VOP_ISLOCKED(attr_sub) == LK_EXCLUSIVE);
error = ufs_extattr_iterate_directory(VFSTOUFS(mp),
attr_sub, namespace, l);
if (error) {
- printf("%s: ufs_extattr_iterate_directory for `%s/%s' (%d)\n",
- __func__, UFS_EXTATTR_FSROOTSUBDIR, subdir, error);
+ printf("%s: ufs_extattr_iterate_directory `%s/%s/%s' (%d)\n",
+ __func__, mp->mnt_stat.f_mntonname,
+ UFS_EXTATTR_FSROOTSUBDIR, subdir, error);
}
KASSERT(VOP_ISLOCKED(attr_sub) == LK_EXCLUSIVE);
vput(attr_sub);
@@ -710,8 +708,8 @@ ufs_extattr_autostart(struct mount *mp,
/* rvp ref'd but now unlocked */
KASSERT(VOP_ISLOCKED(rvp) == 0);
vrele(rvp);
- printf("%s: lookup `%s' (%d)\n", __func__,
- UFS_EXTATTR_FSROOTSUBDIR, error);
+ printf("%s: lookup `%s/%s' (%d)\n", __func__,
+ mp->mnt_stat.f_mntonname, UFS_EXTATTR_FSROOTSUBDIR, error);
return error;
}
if (rvp == attr_dvp) {
@@ -719,8 +717,8 @@ ufs_extattr_autostart(struct mount *mp,
KASSERT(VOP_ISLOCKED(rvp) == LK_EXCLUSIVE);
vrele(attr_dvp);
vput(rvp);
- printf("%s: `/' == `%s' (%d)\n", __func__,
- UFS_EXTATTR_FSROOTSUBDIR, EINVAL);
+ printf("%s: `/' == `%s/%s' (%d)\n", __func__,
+ mp->mnt_stat.f_mntonname, UFS_EXTATTR_FSROOTSUBDIR, EINVAL);
return EINVAL;
}
KASSERT(VOP_ISLOCKED(rvp) == 0);
@@ -729,8 +727,9 @@ ufs_extattr_autostart(struct mount *mp,
KASSERT(VOP_ISLOCKED(attr_dvp) == LK_EXCLUSIVE);
if (attr_dvp->v_type != VDIR) {
- printf("%s: `%s' is not a directory\n",
- __func__, UFS_EXTATTR_FSROOTSUBDIR);
+ printf("%s: `%s/%s' is not a directory\n",
+ __func__, mp->mnt_stat.f_mntonname,
+ UFS_EXTATTR_FSROOTSUBDIR);
goto return_vput_attr_dvp;
}
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: christos Date: Mon Aug 19 14:09:12 UTC 2019 Modified Files: src/sys/ufs/ufs: ufs_extattr.c Log Message: - KNF more - print the full path in error messages To generate a diff of this commit: cvs rdiff -u -r1.49 -r1.50 src/sys/ufs/ufs/ufs_extattr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: christos Date: Mon Aug 19 09:30:30 UTC 2019 Modified Files: src/sys/ufs/ufs: ufs_extattr.c Log Message: - return (foo) -> return foo - normalize all error messages to use __func__ - add more messages for startup failure (should perhaps auto-create the attributes directory and the user and system subdirs?) - factor out common code To generate a diff of this commit: cvs rdiff -u -r1.48 -r1.49 src/sys/ufs/ufs/ufs_extattr.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: christos
Date: Mon Aug 19 09:30:30 UTC 2019
Modified Files:
src/sys/ufs/ufs: ufs_extattr.c
Log Message:
- return (foo) -> return foo
- normalize all error messages to use __func__
- add more messages for startup failure (should perhaps auto-create
the attributes directory and the user and system subdirs?)
- factor out common code
To generate a diff of this commit:
cvs rdiff -u -r1.48 -r1.49 src/sys/ufs/ufs/ufs_extattr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/ufs_extattr.c
diff -u src/sys/ufs/ufs/ufs_extattr.c:1.48 src/sys/ufs/ufs/ufs_extattr.c:1.49
--- src/sys/ufs/ufs/ufs_extattr.c:1.48 Wed Nov 9 00:08:35 2016
+++ src/sys/ufs/ufs/ufs_extattr.c Mon Aug 19 05:30:30 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_extattr.c,v 1.48 2016/11/09 05:08:35 dholland Exp $ */
+/* $NetBSD: ufs_extattr.c,v 1.49 2019/08/19 09:30:30 christos Exp $ */
/*-
* Copyright (c) 1999-2002 Robert N. M. Watson
@@ -48,7 +48,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.48 2016/11/09 05:08:35 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_extattr.c,v 1.49 2019/08/19 09:30:30 christos Exp $");
#ifdef _KERNEL_OPT
#include "opt_ffs.h"
@@ -207,10 +207,10 @@ ufs_extattr_valid_attrname(int attrnames
{
if (attrname == NULL)
- return (0);
+ return 0;
if (strlen(attrname) == 0)
- return (0);
- return (1);
+ return 0;
+ return 1;
}
/*
@@ -321,8 +321,8 @@ ufs_extattr_autocreate_attr(struct vnode
VOP_UNLOCK(backing_vp);
if (error != 0) {
- printf("%s: write uef header failed for %s, error = %d\n",
- __func__, attrname, error);
+ printf("%s: write uef header failed for `%s' (%d)\n",
+ __func__, attrname, error);
vn_close(backing_vp, FREAD|FWRITE, l->l_cred);
*uelep = NULL;
return error;
@@ -335,8 +335,8 @@ ufs_extattr_autocreate_attr(struct vnode
KASSERT(VOP_ISLOCKED(backing_vp) == 0);
if (error != 0) {
- printf("%s: enable %s failed, error %d\n",
- __func__, attrname, error);
+ printf("%s: enable `%s' failed (%d)\n",
+ __func__, attrname, error);
vn_close(backing_vp, FREAD|FWRITE, l->l_cred);
*uelep = NULL;
return error;
@@ -344,7 +344,7 @@ ufs_extattr_autocreate_attr(struct vnode
uele = ufs_extattr_find_attr(ump, attrnamespace, attrname);
if (uele == NULL) {
- printf("%s: atttribute %s created but not found!\n",
+ printf("%s: atttribute `%s' created but not found!\n",
__func__, attrname);
vn_close(backing_vp, FREAD|FWRITE, l->l_cred);
*uelep = NULL;
@@ -352,7 +352,7 @@ ufs_extattr_autocreate_attr(struct vnode
}
printf("%s: EA backing store autocreated for %s\n",
- mp->mnt_stat.f_mntonname, attrname);
+ mp->mnt_stat.f_mntonname, attrname);
*uelep = uele;
return 0;
@@ -374,11 +374,11 @@ ufs_extattr_find_attr(struct ufsmount *u
if (!(strncmp(attrname, search_attribute->uele_attrname,
UFS_EXTATTR_MAXEXTATTRNAME)) &&
(attrnamespace == search_attribute->uele_attrnamespace)) {
- return (search_attribute);
+ return search_attribute;
}
}
- return (0);
+ return 0;
}
/*
@@ -453,8 +453,7 @@ ufs_extattr_start(struct mount *mp, stru
unlock:
ufs_extattr_uepm_unlock(ump);
-
- return (error);
+ return error;
}
/*
@@ -490,8 +489,8 @@ ufs_extattr_lookup(struct vnode *start_d
VOP_UNLOCK(start_dvp);
}
PNBUF_PUT(pnbuf);
- printf("ufs_extattr_lookup: copystr failed\n");
- return (error);
+ printf("%s: copystr failed (%d)\n", __func__, error);
+ return error;
}
cnp.cn_namelen--; /* trim nul termination */
vargs.a_desc = NULL;
@@ -504,11 +503,11 @@ ufs_extattr_lookup(struct vnode *start_d
if (lockparent == 0) {
VOP_UNLOCK(start_dvp);
}
- return (error);
+ return error;
}
#if 0
if (target_vp == start_dvp)
- panic("ufs_extattr_lookup: target_vp == start_dvp");
+ panic("%s: target_vp == start_dvp", __func__);
#endif
if (target_vp != start_dvp) {
@@ -523,7 +522,7 @@ ufs_extattr_lookup(struct vnode *start_d
KASSERT(VOP_ISLOCKED(target_vp) == LK_EXCLUSIVE);
*vp = target_vp;
- return (0);
+ return 0;
}
/*
@@ -541,10 +540,9 @@ ufs_extattr_enable_with_open(struct ufsm
error = VOP_OPEN(vp, FREAD|FWRITE, l->l_cred);
if (error) {
- printf("ufs_extattr_enable_with_open.VOP_OPEN(): failed "
- "with %d\n", error);
+ printf("%s: VOP_OPEN(): failed (%d)\n", __func__, error);
VOP_UNLOCK(vp);
- return (error);
+ return error;
}
mutex_enter(vp->v_interlock);
@@ -558,7 +556,7 @@ ufs_extattr_enable_with_open(struct ufsm
error = ufs_extattr_enable(ump, attrnamespace, attrname, vp, l);
if (error != 0)
vn_close(vp, FREAD|FWRITE, l->l_cred);
- return (error);
+ return error;
}
/*
@@ -582,7 +580,7 @@ ufs_extattr_iterate_directory(struct ufs
int error, eofflag = 0;
if (dvp->v_type != VDIR)
- return (ENOTDIR);
+ return ENOTDIR;
dirbuf
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: dholland Date: Mon Jul 1 00:57:06 UTC 2019 Modified Files: src/sys/ufs/ufs: ufs_vnops.c Log Message: Lay down some comments related to the previous few revisions of ufs_vnops.c. To generate a diff of this commit: cvs rdiff -u -r1.246 -r1.247 src/sys/ufs/ufs/ufs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: dholland Date: Mon Jul 1 00:57:06 UTC 2019 Modified Files: src/sys/ufs/ufs: ufs_vnops.c Log Message: Lay down some comments related to the previous few revisions of ufs_vnops.c. To generate a diff of this commit: cvs rdiff -u -r1.246 -r1.247 src/sys/ufs/ufs/ufs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/ufs/ufs/ufs_vnops.c diff -u src/sys/ufs/ufs/ufs_vnops.c:1.246 src/sys/ufs/ufs/ufs_vnops.c:1.247 --- src/sys/ufs/ufs/ufs_vnops.c:1.246 Mon Feb 25 06:00:40 2019 +++ src/sys/ufs/ufs/ufs_vnops.c Mon Jul 1 00:57:06 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_vnops.c,v 1.246 2019/02/25 06:00:40 dholland Exp $ */ +/* $NetBSD: ufs_vnops.c,v 1.247 2019/07/01 00:57:06 dholland Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -66,7 +66,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.246 2019/02/25 06:00:40 dholland Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.247 2019/07/01 00:57:06 dholland Exp $"); #if defined(_KERNEL_OPT) #include "opt_ffs.h" @@ -1257,7 +1257,12 @@ ufs_readdir(void *v) KASSERT(VOP_ISLOCKED(vp)); - /* figure out where we want to read */ + /* + * Figure out where the user wants us to read and how much. + * + * XXX: there should probably be an upper bound on callerbytes + * to avoid silliness trying to do large kernel allocations. + */ callerbytes = calleruio->uio_resid; startoffset = calleruio->uio_offset; endoffset = startoffset + callerbytes; @@ -1267,7 +1272,39 @@ ufs_readdir(void *v) return EINVAL; } - /* round start and end down to block boundaries */ + /* + * Now figure out where to actually start reading. Round the + * start down to a block boundary: we need to start at the + * beginning of a block in order to read the directory + * correctly. + * + * We also want to always read a whole number of blocks so + * that the copying code below doesn't have to worry about + * partial entries. (It used to try at one point, and was a + * horrible mess.) + * + * Furthermore, since blocks have to be scanned from the + * beginning, if we go partially into another block now we'll + * just have to rescan it on the next readdir call, which + * doesn't really serve any useful purpose. + * + * So, round down the end as well. It's ok to underpopulate + * the transfer buffer, as long as we send back at least one + * dirent so as to avoid giving a bogus EOF indication. + * + * Note that because dirents are larger than ffs struct + * directs, despite the rounding down we may not be able to + * send all the entries in the blocks we read and may have to + * rescan some of them on the next call anyway. Alternatively + * if there's empty space on disk we might have actually been + * able to fit the next block in, and so forth. None of this + * actually matters that much in practice. + * + * XXX: what does ffs do if a directory block becomes + * completely empty, and what happens if all the blocks we + * read are completely empty even though we aren't at EOF? As + * of this writing I (dholland) can't remember the details. + */ physstart = rounddown2(startoffset, ump->um_dirblksiz); physend = rounddown2(endoffset, ump->um_dirblksiz); @@ -1276,10 +1313,42 @@ ufs_readdir(void *v) return EINVAL; } + /* + * skipstart is the number of bytes we need to read in + * (because we need to start at the beginning of a block) but + * not transfer to the user. + * + * dropend is the number of bytes to ignore at the end of the + * user's buffer. + */ skipstart = startoffset - physstart; dropend = endoffset - physend; - /* how much to actually read */ + /* + * Make a transfer buffer. + * + * Note: rawbufmax = physend - physstart. Proof: + * + * physend - physstart = physend - physstart + * = physend - physstart + startoffset - startoffset + * = physend + (startoffset - physstart) - startoffset + * = physend + skipstart - startoffset + * = physend + skipstart - startoffset + endoffset - endoffset + * = skipstart - startoffset + endoffset - (endoffset - physend) + * = skipstart - startoffset + endoffset - dropend + * = skipstart - startoffset + (startoffset + callerbytes) - dropend + * = skipstart + callerbytes - dropend + * = rawbufmax + * Qed. + * + * XXX: this should just use physend - physstart. + * + * XXX: this should be rewritten to read the directs straight + * out of bufferio buffers instead of copying twice. This would + * also let us adapt better to the user's buffer size. + */ + + /* Base buffer space for CALLERBYTES of new data */ rawbufmax = callerbytes + skipstart; if (rawbufmax < callerbytes) return EINVAL;
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: christos Date: Thu Jun 20 00:52:05 UTC 2019 Modified Files: src/sys/ufs/ufs: ufs_readwrite.c Log Message: unifdef -ULFS_READWRITE ufs_readwrite.c To generate a diff of this commit: cvs rdiff -u -r1.123 -r1.124 src/sys/ufs/ufs/ufs_readwrite.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src
Committed By: christos
Date: Thu Jun 20 00:52:05 UTC 2019
Modified Files:
src/sys/ufs/ufs: ufs_readwrite.c
Log Message:
unifdef -ULFS_READWRITE ufs_readwrite.c
To generate a diff of this commit:
cvs rdiff -u -r1.123 -r1.124 src/sys/ufs/ufs/ufs_readwrite.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/ufs/ufs/ufs_readwrite.c
diff -u src/sys/ufs/ufs/ufs_readwrite.c:1.123 src/sys/ufs/ufs/ufs_readwrite.c:1.124
--- src/sys/ufs/ufs/ufs_readwrite.c:1.123 Mon Dec 10 15:48:34 2018
+++ src/sys/ufs/ufs/ufs_readwrite.c Wed Jun 19 20:52:05 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ufs_readwrite.c,v 1.123 2018/12/10 20:48:34 jdolecek Exp $ */
+/* $NetBSD: ufs_readwrite.c,v 1.124 2019/06/20 00:52:05 christos Exp $ */
/*-
* Copyright (c) 1993
@@ -32,28 +32,8 @@
*/
#include
-__KERNEL_RCSID(1, "$NetBSD: ufs_readwrite.c,v 1.123 2018/12/10 20:48:34 jdolecek Exp $");
+__KERNEL_RCSID(1, "$NetBSD: ufs_readwrite.c,v 1.124 2019/06/20 00:52:05 christos Exp $");
-#ifdef LFS_READWRITE
-#define FS struct lfs
-#define I_FS i_lfs
-#define READ lfs_read
-#define READ_S "lfs_read"
-#define WRITE lfs_write
-#define WRITE_S "lfs_write"
-#define BUFRD lfs_bufrd
-#define BUFWR lfs_bufwr
-#define fs_bsize lfs_bsize
-#define fs_bmask lfs_bmask
-#define UFS_WAPBL_BEGIN(mp) 0
-#define UFS_WAPBL_END(mp) do { } while (0)
-#define UFS_WAPBL_UPDATE(vp, access, modify, flags) do { } while (0)
-#define ufs_blkoff lfs_blkoff
-#define ufs_blksize lfs_blksize
-#define ufs_lblkno lfs_lblkno
-#define ufs_lblktosize lfs_lblktosize
-#define ufs_blkroundup lfs_blkroundup
-#else
#define FS struct fs
#define I_FS i_fs
#define READ ffs_read
@@ -67,7 +47,6 @@ __KERNEL_RCSID(1, "$NetBSD: ufs_readwrit
#define ufs_lblkno ffs_lblkno
#define ufs_lblktosize ffs_lblktosize
#define ufs_blkroundup ffs_blkroundup
-#endif
static int ufs_post_read_update(struct vnode *, int, int);
static int ufs_post_write_update(struct vnode *, struct uio *, int,
@@ -106,20 +85,13 @@ READ(void *v)
/* XXX Eliminate me by refusing directory reads from userland. */
if (vp->v_type == VDIR)
return BUFRD(vp, uio, ioflag, ap->a_cred);
-#ifdef LFS_READWRITE
- /* XXX Eliminate me by using ufs_bufio in lfs. */
- if (vp->v_type == VREG && ip->i_number == LFS_IFILE_INUM)
- return BUFRD(vp, uio, ioflag, ap->a_cred);
-#endif
if ((u_int64_t)uio->uio_offset > ump->um_maxfilesize)
return (EFBIG);
if (uio->uio_resid == 0)
return (0);
-#ifndef LFS_READWRITE
if ((ip->i_flags & (SF_SNAPSHOT | SF_SNAPINVAL)) == SF_SNAPSHOT)
return ffs_snapshot_read(vp, uio, ioflag);
-#endif /* !LFS_READWRITE */
if (uio->uio_offset >= ip->i_size)
goto out;
@@ -177,9 +149,7 @@ BUFRD(struct vnode *vp, struct uio *uio,
if (uio->uio_resid == 0)
return 0;
-#ifndef LFS_READWRITE
KASSERT(!ISSET(ip->i_flags, (SF_SNAPSHOT | SF_SNAPINVAL)));
-#endif
if (uio->uio_offset >= ip->i_size)
goto out;
@@ -302,12 +272,6 @@ WRITE(void *v)
if (uio->uio_offset < 0 ||
(u_int64_t)uio->uio_offset + uio->uio_resid > ump->um_maxfilesize)
return (EFBIG);
-#ifdef LFS_READWRITE
- /* Disallow writes to the Ifile, even if noschg flag is removed */
- /* XXX can this go away when the Ifile is no longer in the namespace? */
- if (vp == fs->lfs_ivnode)
- return (EPERM);
-#endif
if (uio->uio_resid == 0)
return (0);
@@ -352,11 +316,6 @@ WRITE(void *v)
return error;
}
-#ifdef LFS_READWRITE
- async = true;
- lfs_availwait(fs, btofsb(fs, uio->uio_resid));
- lfs_check(vp, LFS_UNUSED_LBN, 0);
-#endif /* !LFS_READWRITE */
preallocoff = round_page(ufs_blkroundup(fs, MAX(osize, uio->uio_offset)));
aflag = ioflag & IO_SYNC ? B_SYNC : 0;
@@ -471,7 +430,6 @@ WRITE(void *v)
* XXXUBC simplistic async flushing.
*/
-#ifndef LFS_READWRITE
if (!async && oldoff >> 16 != uio->uio_offset >> 16) {
mutex_enter(vp->v_interlock);
error = VOP_PUTPAGES(vp, (oldoff >> 16) << 16,
@@ -480,7 +438,6 @@ WRITE(void *v)
if (error)
break;
}
-#endif
}
if (error == 0 && ioflag & IO_SYNC) {
mutex_enter(vp->v_interlock);
@@ -513,9 +470,6 @@ BUFWR(struct vnode *vp, struct uio *uio,
daddr_t lbn;
int extended=0;
int error;
-#ifdef LFS_READWRITE
- bool need_unreserve = false;
-#endif
KASSERT(ISSET(ioflag, IO_NODELOCKED));
KASSERT(VOP_ISLOCKED(vp) == LK_EXCLUSIVE);
@@ -535,9 +489,6 @@ BUFWR(struct vnode *vp, struct uio *uio,
uio->uio_resid > ump->um_maxfilesize ||
uio->uio_offset > (ump->um_maxfilesize - uio->uio_resid))
return EFBIG;
-#ifdef LFS_READWRITE
- KASSERT(vp != fs->lfs_ivnode);
-#endif
if (uio->uio_resid == 0)
return 0;
@@ -548,10 +499,6 @@ BUFWR(struct vnode *vp, struct uio *uio,
KASSERT(vp->v_type != VREG);
-#ifdef LFS_READWRITE
- lfs_availwait(fs, btofsb(fs, uio->uio_resid));
- lfs_check(vp, LFS_UNUSED_LBN, 0);
-#endif /* !LFS_READWRITE */
/* XXX
Re: CVS commit: src/sys/ufs/ufs
On Sun, Feb 24, 2019 at 10:13:40PM +0100, Tobias Nygren wrote: > On Sun, 24 Feb 2019 19:06:40 + > Michael van Elst wrote: > > > To generate a diff of this commit: > > cvs rdiff -u -r1.242 -r1.243 src/sys/ufs/ufs/ufs_vnops.c > > > + rawbuf -= dropend; > > I guess this should be rawbufmax, not rawbuf. Yes, already fixed. -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/ufs/ufs
On Mon, Feb 25, 2019 at 06:07:23AM +, David Holland wrote:
> that one doesn't set dropend correctly for small buffers, outsmarted
> myself while writing it.
Better change (still against 1.242) that makes the logic much
simpler. Will test this overnight...
Index: ufs_vnops.c
===
RCS file: /cvsroot/src/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.239
diff -u -p -r1.239 ufs_vnops.c
--- ufs_vnops.c 28 Oct 2017 00:37:13 - 1.239
+++ ufs_vnops.c 25 Feb 2019 06:58:52 -
@@ -1233,7 +1233,7 @@ ufs_readdir(void *v)
#endif
/* caller's buffer */
struct uio *calleruio = ap->a_uio;
- off_t startoffset, endoffset;
+ off_t startoffset;
size_t callerbytes;
off_t curoffset;
/* dirent production buffer */
@@ -1244,8 +1244,8 @@ ufs_readdir(void *v)
off_t *cookies;
size_t numcookies, maxcookies;
/* disk buffer */
- off_t physstart, physend;
- size_t skipstart, dropend;
+ off_t physstart;
+ size_t skipstart;
char*rawbuf;
size_t rawbufmax, rawbytes;
struct uio rawuio;
@@ -1256,29 +1256,60 @@ ufs_readdir(void *v)
KASSERT(VOP_ISLOCKED(vp));
- /* figure out where we want to read */
+#define UFS_READDIR_ARBITRARY_MAX (1024*1024)
callerbytes = calleruio->uio_resid;
- startoffset = calleruio->uio_offset;
- endoffset = startoffset + callerbytes;
-
if (callerbytes < _DIRENT_MINSIZE(dirent)) {
/* no room for even one struct dirent */
return EINVAL;
}
+ if (callerbytes > UFS_READDIR_ARBITRARY_MAX) {
+ callerbytes = UFS_READDIR_ARBITRARY_MAX;
+ }
- /* round start and end down to block boundaries */
+ /*
+* Figure out where to start reading. Round the start down to
+* a block boundary: we need to start at the beginning of a
+* block in order to read the directory correctly.
+*
+* skipstart is the number of bytes we need to read in
+* (because we need to start at the beginning of a block) but
+* not transfer to the user.
+*/
+ startoffset = calleruio->uio_offset;
physstart = startoffset & ~(off_t)(ump->um_dirblksiz - 1);
- physend = endoffset & ~(off_t)(ump->um_dirblksiz - 1);
skipstart = startoffset - physstart;
- dropend = endoffset - physend;
- if (callerbytes - dropend < _DIRENT_MINSIZE(rawdp)) {
- /* no room for even one struct direct */
- return EINVAL;
- }
+ /*
+* Now figure out how much to read.
+*
+* callerbytes tells us how much data we can send back to the
+* user. Assume as a starting point that we want to read
+* roughly the same volume of struct directs from disk as the
+* volume of struct dirents we want to send to the user; this
+* is not super accurate for large numbers of small names but
+* will serve well enough. It's ok to underpopulate the user's
+* buffer, and most directories are only a couple blocks
+* anyway.
+*
+* However much we decide we want, stop on a block boundary.
+* That way the copying code below doesn't need to worry about
+* finding partial entries in the transfer buffer.
+*
+* Do this by rounding down (not up) by default as that will
+* with luck avoid needing to scan the next block twice; but
+* always read in at least one block.
+*/
- /* how much to actually read */
- rawbufmax = callerbytes + skipstart - dropend;
+ /* Base buffer space for CALLERBYTES of new data */
+ rawbufmax = callerbytes + skipstart;
+
+ /* Round down to an integral number of blocks */
+ rawbufmax &= ~(off_t)(ump->um_dirblksiz - 1);
+
+ /* but always at least one block */
+ if (rawbufmax == 0) {
+ rawbufmax = ump->um_dirblksiz;
+ }
/* read it */
rawbuf = kmem_alloc(rawbufmax, KM_SLEEP);
--
David A. Holland
dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Mon, Feb 25, 2019 at 05:50:08AM +, David Holland wrote: > Furthermore, this: > > > + rawbuf -= dropend; > > is entirely wrong (it needs to be "rawbufmax") and without that bound > on rawbufmax the code is unsafe... I repaired this bit just now, so it's not an overt hazard any more. I still don't like this change all that much but whatever, I suppose... > Here's the fix I got bogged down trying to build and test, which also > adds a missing upper bound on callerbytes: that one doesn't set dropend correctly for small buffers, outsmarted myself while writing it. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sun, Feb 24, 2019 at 07:51:24PM -0500, Christos Zoulas wrote: > Module Name: src > Committed By:christos > Date:Mon Feb 25 00:51:24 UTC 2019 > > Modified Files: > src/sys/ufs/ufs: ufs_vnops.c > > Log Message: > drop unused dropping this logic is wrong... -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sun, 24 Feb 2019 19:06:40 + Michael van Elst wrote: > To generate a diff of this commit: > cvs rdiff -u -r1.242 -r1.243 src/sys/ufs/ufs/ufs_vnops.c > + rawbuf -= dropend; I guess this should be rawbufmax, not rawbuf.
Re: CVS commit: src/sys/ufs/ufs
On Sun, Feb 24, 2019 at 07:41:59PM +, m...@netbsd.org wrote: > something like the overflow is undefined behaviour, so it cannot > happen, so the branch checking that it happened is eliminated. *Signed* integer overflow is undefined behavior. *Unsigned* integer overflow is well defined. Joerg
Re: CVS commit: src/sys/ufs/ufs
On Sun, Feb 24, 2019 at 07:41:59PM +, m...@netbsd.org wrote: > something like the overflow is undefined behaviour, so it cannot > happen, so the branch checking that it happened is eliminated. Integer overflow is not undefined, but implemenation defined behaviour. Martin
Re: CVS commit: src/sys/ufs/ufs
On Sun, Feb 24, 2019 at 07:06:40PM +, Michael van Elst wrote: > While here, also check for arithmetic overflow. > + /* how much to actually read */ > + rawbufmax = callerbytes + skipstart; > + if (rawbufmax < callerbytes) > + return EINVAL; hmm, I"m under the impression that checking for overflow without upsetting the compiler is a delicate matter. something like the overflow is undefined behaviour, so it cannot happen, so the branch checking that it happened is eliminated.
Re: CVS commit: src/sys/ufs/ufs
Indeed rebooting with an updated kernel will give active NFS clients problems, but I am not sure we should realy care nor how we could possibly avoid this one time issue. We have changed encoding of filehandles before (at least once). it's a bad thing and worth mentioning in release notes. how about adding a version field so that it won't happen again? YAMAMOTO Takashi
Re: CVS commit: src/sys/ufs/ufs
Hi David Holland: I'm not entirely clear on how this structure is actually used, which is why I hadn't yet made this change myself after the issue came up. :-/ My observation in sys/fs/cd9660 is probably the initial cause for this change. PR kern/48799. I could produce a problem by exporting to NFS an ISO-9960 filesystem which gets 64-bit inodes due to high storage loacations of its directory records. NFS uses these structs to identify files in the original filesystem. cd9660 then uses the struct ifid member ifid_ino to fulfill this wish ... if no 32-bit bottleneck prevents it. So if the fix in ufs causes problems, could you please also have a look at the fix of kern/48799 ? Have a nice day :) Thomas
Re: CVS commit: src/sys/ufs/ufs
On Fri, May 16, 2014 at 12:12:00AM +0200, Joerg Sonnenberger wrote: I don't think it is a problem by itself as the file handle is opaque, but it is certainly wasteful to not put ufid_ino first. That is not possible, the first few bytes of the structure must match struct fid from fstypes.h. The structure is only used to construct opaque tokens for filehandles, and the encoding includes the size, so I see no problem. Indeed rebooting with an updated kernel will give active NFS clients problems, but I am not sure we should realy care nor how we could possibly avoid this one time issue. We have changed encoding of filehandles before (at least once). Martin
Re: CVS commit: src/sys/ufs/ufs
On Fri, May 16, 2014 at 03:54:44PM +, David Holland wrote: Indeed rebooting with an updated kernel will give active NFS clients problems, but I am not sure we should realy care nor how we could possibly avoid this one time issue. We have changed encoding of filehandles before (at least once). I don't think this is a problem, but maybe I'll put a note in UPDATING. Never mind that problem. Consider what happens if you reboot with a different CD in the drive! I once fixed a filesystem to use different faked inode numbers every time a filesystem was mounted. Without that NFS clients would write to the wrong file in the wrong FS. The 'impossible to get rid of' retries for hard mounts were something up with which I had to put. (A preposition is something you should not end a sentence with.) David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/ufs/ufs
On Fri, May 16, 2014 at 06:48:06PM +, Martin Husemann wrote: The problem is that the tokens are memcmp'd, so if they include struct padding this may not work. All filesystems I've seen init the struct with a full memset to 0, so all padding fields should be initialized as well. Ok, but... However, we can swap the last two fields in the ufid case, if you prefer. ...may as well, I guess. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Wed, May 14, 2014 at 01:46:19PM +, Martin Husemann wrote: Modified Files: src/sys/ufs/ufs: inode.h Log Message: Make filehandles on UFS based filesystems use proper 64bit inodes. 32bit restriction noticed by Taylor R Campbell. I suspect this isn't going to work: u_int16_t ufid_len; /* Length of structure. */ u_int16_t ufid_pad; /* Force 32-bit alignment. */ ino_t ufid_ino; /* File number (ino). */ int32_t ufid_gen; /* Generation number. */ Probably it should be specifically sized, therefore uint64_t; also it needs padding for 64-bit alignment. I'm not entirely clear on how this structure is actually used, which is why I hadn't yet made this change myself after the issue came up. :-/ -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Thu, May 15, 2014 at 10:06:18PM +, David Holland wrote: On Wed, May 14, 2014 at 01:46:19PM +, Martin Husemann wrote: Modified Files: src/sys/ufs/ufs: inode.h Log Message: Make filehandles on UFS based filesystems use proper 64bit inodes. 32bit restriction noticed by Taylor R Campbell. I suspect this isn't going to work: I don't think it is a problem by itself as the file handle is opaque, but it is certainly wasteful to not put ufid_ino first. Joerg
Re: CVS commit: src/sys/ufs/ufs
On Fri, May 16, 2014 at 12:12:00AM +0200, Joerg Sonnenberger wrote: Modified Files: src/sys/ufs/ufs: inode.h Log Message: Make filehandles on UFS based filesystems use proper 64bit inodes. 32bit restriction noticed by Taylor R Campbell. I suspect this isn't going to work: I don't think it is a problem by itself as the file handle is opaque, but it is certainly wasteful to not put ufid_ino first. I've ascertained that it is in fact a problem: this thing is sent directly on the wire, and it gets memcmp'd, so having padding bytes in it is going to cause trouble. Also, if you reboot and update the kernel to add this change all old file handles will quit working; this probably doesn't matter though. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sun, Jan 29, 2012 at 08:49:02AM +, Izumi Tsutsui wrote: Modified Files: src/sys/ufs/ufs: ufs_vfsops.c Log Message: Fix errors in !defined(QUOTA) !defined(QUOTA2) case. Whoops, sorry... -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Tue, Oct 11, 2011 at 04:40:02AM +, David Holland wrote: On Sun, Oct 09, 2011 at 09:15:34PM +, Chuck Silvers wrote: Modified Files: src/sys/ufs/ufs: extattr.h Log Message: add forward declarations for the VOP args structures so that fstat can include this file. Do we really want fstat using fs-specific headers and interfaces? (I know, cleaning it up is probably nontrivial) sure, it'd be better if fstat used sysctl instead of libkvm to extract all this info from the kernel, but as you say, non-trivial. -Chuck
Re: CVS commit: src/sys/ufs/ufs
On Sun, Oct 09, 2011 at 09:15:34PM +, Chuck Silvers wrote: Modified Files: src/sys/ufs/ufs: extattr.h Log Message: add forward declarations for the VOP args structures so that fstat can include this file. Do we really want fstat using fs-specific headers and interfaces? (I know, cleaning it up is probably nontrivial) -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sun, Jul 17, 2011 at 10:02:27PM +, David A. Holland wrote: Modified Files: src/sys/ufs/ufs: ufs_vnops.c Log Message: At the end of ufs_rmdir, don't use a dangling vnode pointer to call fstrans_done. Ok hannken@ Well, for the record it turns out I misread his mail and maybe this isn't ok... will fix. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Tue, 25 May 2010, Antti Kantee wrote: What would I have to do from userland to tickle this bug? You need to unlink and create a file after the first namei in sys_rename but before VOP_RENAME runs, i.e. trigger a race condition. Thanks. --apb (Alan Barrett)
Re: CVS commit: src/sys/ufs/ufs
On Tue, 25 May 2010, Antti Kantee wrote: Modified Files: src/sys/ufs/ufs: ufs_wapbl.c Log Message: Add a comment describing an observed boom-crash-burn problem in the code. Fixing it will require a full tank of gas, half a pack of cigarettes, sunglasses, darkness, and most importantly: someone else. What would I have to do from userland to tickle this bug? --apb (Alan Barrett)
Re: CVS commit: src/sys/ufs/ufs
On Tue May 25 2010 at 15:55:35 +0200, Alan Barrett wrote: On Tue, 25 May 2010, Antti Kantee wrote: Modified Files: src/sys/ufs/ufs: ufs_wapbl.c Log Message: Add a comment describing an observed boom-crash-burn problem in the code. Fixing it will require a full tank of gas, half a pack of cigarettes, sunglasses, darkness, and most importantly: someone else. What would I have to do from userland to tickle this bug? You need to unlink and create a file after the first namei in sys_rename but before VOP_RENAME runs, i.e. trigger a race condition. Running tests/fs/ffs/t_renamerace a few times should suffice. It uses rump, so your host is safe. If you have a uniprocessor host, set RUMP_NCPU to 2 to have both threads run in the rump kernel in parallel (well, they run virtually parallel on a uniprocessor system, but YKWIM). This makes the race more likely to trigger.
Re: CVS commit: src/sys/ufs/ufs
On Tue, May 25, 2010 at 11:02:07AM +, Antti Kantee wrote: Add a comment describing an observed boom-crash-burn problem in the code. Fixing it will require a full tank of gas, half a pack of cigarettes, sunglasses, darkness, and most importantly: someone else. Thanks for the merge conflict :-p -- David A. Holland dholl...@netbsd.org
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: pooka Date: Tue Mar 2 14:45:56 UTC 2010 Modified Files: src/sys/ufs/ufs: ufs_wapbl.c Log Message: scortch ufs_vnops.c cargo cult headers To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/ufs/ufs/ufs_wapbl.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/ufs/ufs/ufs_wapbl.c diff -u src/sys/ufs/ufs/ufs_wapbl.c:1.7 src/sys/ufs/ufs/ufs_wapbl.c:1.8 --- src/sys/ufs/ufs/ufs_wapbl.c:1.7 Mon Apr 6 14:09:57 2009 +++ src/sys/ufs/ufs/ufs_wapbl.c Tue Mar 2 14:45:55 2010 @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_wapbl.c,v 1.7 2009/04/06 14:09:57 pooka Exp $ */ +/* $NetBSD: ufs_wapbl.c,v 1.8 2010/03/02 14:45:55 pooka Exp $ */ /*- * Copyright (c) 2003,2006,2008 The NetBSD Foundation, Inc. @@ -66,12 +66,7 @@ */ #include sys/cdefs.h -__KERNEL_RCSID(0, $NetBSD: ufs_wapbl.c,v 1.7 2009/04/06 14:09:57 pooka Exp $); - -#if defined(_KERNEL_OPT) -#include opt_quota.h -#include fs_lfs.h -#endif +__KERNEL_RCSID(0, $NetBSD: ufs_wapbl.c,v 1.8 2010/03/02 14:45:55 pooka Exp $); #include sys/param.h #include sys/systm.h
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: pooka Date: Tue Mar 2 14:45:56 UTC 2010 Modified Files: src/sys/ufs/ufs: ufs_wapbl.c Log Message: scortch ufs_vnops.c cargo cult headers To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 src/sys/ufs/ufs/ufs_wapbl.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/ufs/ufs
Module Name:src Committed By: bouyer Date: Fri Jan 15 19:46:35 UTC 2010 Modified Files: src/sys/ufs/ufs: ufs_quota.c Log Message: vclean() actually sets v_tag to VT_NON but doesn't touch v_type. getcleanvnode() sets v_type to VNON after releasing v_interlock. So the thread doing quotaon(), quotaoff() or qsync() could vget() a vnode which is being recycled in getcleanvnode(), after is has been cleaned and v_interlock released, but before v_type has been reset, leading to KASSERT(vp-v_usecount == 1) firing in getnewvnode(), or qsync() dereferending a NULL pointer as in PR kern/42205. Fix by using the same tests as other ffs function traversing the mount list: also check for VTOI(vp) == NULL, and VI_XLOCK in addition to VI_CLEAN. To generate a diff of this commit: cvs rdiff -u -r1.64 -r1.65 src/sys/ufs/ufs/ufs_quota.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/ufs/ufs
On Tue, Sep 22, 2009 at 10:42:59PM +0300, Antti Kantee wrote: Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? It's becoming clear that this is something I'm going to need to wade into. Much as I've been trying to avoid it. :-/ There's a (perfectly natural) tendency to try to fix synchronization problems by adding states -- extra flags, more locks, moving things to the background, etc. -- but in general the way to fix synchronization problems so they *stay* fixed is to remove states. For example, from what I've seen so far I'm pretty sure XLOCK ought to go away. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/ufs/ufs
On Sep,Tuesday 22 2009, at 9:42 PM, Antti Kantee wrote: On Tue Sep 22 2009 at 21:04:14 +0200, Manuel Bouyer wrote: that's not an issue with the reference count. It's an issue with vclean() calling VOP_RECLAIM() even if the refcount is greater than 1, and vrelel() calling vclean() even if the refcount is greater than 1, when the file has been unlink(2)ed. There's a comment about this in vrelel(), look for variable recycle. Ah, ic, probably would've been easier if I read the PR first ;) So basically someone can vget the vnode (via fhtovp, since it's gone from the directory namespace) between VOP_INACTIVE and clean. Your fix doesn't really fix this problem, since depending on timings the inode might still be recycled after you check it's valid. Hmm, ok, so things which might happen: 1: VOP_REMOVE, vnode is locked, vrele is called 2: fhtovp before inode is reclaimed, blocks on vn_lock 1: VOP_INACTIVE releases vnlock, returns signal to recycle vnode 2: gets lock, does check that it's still the same inode 1: reclaims vnode 2: boom Since vget takes the interlock, a dirty cheap trick might be to check that the reference count is still one before trying to clean the vnode in vrelel(), otherwise punting and letting the next release-to-0 reclaim it? Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? Sorry I haven't read this thread until today. I think that it would be good to test my vreclaim patched kernel it might help to resolve your problem. But I don't think it is a solution see above. Regards Adam.
Re: CVS commit: src/sys/ufs/ufs
On Wed, Sep 23, 2009 at 06:09:00PM +0200, Adam Hamsik wrote: Sorry I haven't read this thread until today. I think that it would be good to test my vreclaim patched kernel it might help to resolve your problem. But I don't think it is a solution see above. Your patch won't help. It only changes the behavior or getnewvnode(), and in this problem getnewvnode() isn't involved at all. Also I can't see how your patch would help, a vnode for an inode which has been deleted still needs to be invalidated at unlink time, and can't be deffered to another thread. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Wed, Sep 23, 2009 at 03:08:14PM +, David Holland wrote: It's becoming clear that this is something I'm going to need to wade into. Much as I've been trying to avoid it. :-/ There's a (perfectly natural) tendency to try to fix synchronization problems by adding states -- extra flags, more locks, moving things to the background, etc. -- but in general the way to fix synchronization problems so they *stay* fixed is to remove states. For example, from what I've seen so far I'm pretty sure XLOCK ought to go away. That's possible, at last in its current form as it fails to protect vget(). -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Sun, Sep 20, 2009 at 08:23:38PM +0300, Antti Kantee wrote: In ufs_ihashget(), vget() can return a vnode that has been vclean'ed because vget() can sleep. After vget returns, check that vp is still connected with ip, and that ip still points to the inode we want. This fix the NULL pointer dereference in ufs_fhtovp() I've been seeing on a NFS server. Um, hold the phone. The whole point of vget() is to provide race-free access to the weak vnode reference held by the file system. Are you saying this does not hold anymore? It depends on what you mean with race-free. If you mean that the vnode returned by vget() can't be recygled, I think this is true. If you mean that vget() can't return a clean vnode then this is false: vget() can sleep in vn_lock(), and it releases the v_interlock mutex before sleeping. While sleeping vclean() can VOP_RECLAIM() the vnode, even if v_usecount is 1. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Tue Sep 22 2009 at 20:06:40 +0200, Manuel Bouyer wrote: On Sun, Sep 20, 2009 at 08:23:38PM +0300, Antti Kantee wrote: In ufs_ihashget(), vget() can return a vnode that has been vclean'ed because vget() can sleep. After vget returns, check that vp is still connected with ip, and that ip still points to the inode we want. This fix the NULL pointer dereference in ufs_fhtovp() I've been seeing on a NFS server. Um, hold the phone. The whole point of vget() is to provide race-free access to the weak vnode reference held by the file system. Are you saying this does not hold anymore? It depends on what you mean with race-free. If you mean that the vnode returned by vget() can't be recygled, I think this is true. If you mean that vget() can't return a clean vnode then this is false: vget() can sleep in vn_lock(), and it releases the v_interlock mutex before sleeping. While sleeping vclean() can VOP_RECLAIM() the vnode, even if v_usecount is 1. What is the practical difference of cleaned and recycled for the file system driver? If there is a race in vfs and XLOCK is not used properly, I think that should be investigated and fixed instead of patching file systems here and there.
Re: CVS commit: src/sys/ufs/ufs
On Tue, Sep 22, 2009 at 09:23:05PM +0300, Antti Kantee wrote: On Tue Sep 22 2009 at 20:06:40 +0200, Manuel Bouyer wrote: On Sun, Sep 20, 2009 at 08:23:38PM +0300, Antti Kantee wrote: In ufs_ihashget(), vget() can return a vnode that has been vclean'ed because vget() can sleep. After vget returns, check that vp is still connected with ip, and that ip still points to the inode we want. This fix the NULL pointer dereference in ufs_fhtovp() I've been seeing on a NFS server. Um, hold the phone. The whole point of vget() is to provide race-free access to the weak vnode reference held by the file system. Are you saying this does not hold anymore? It depends on what you mean with race-free. If you mean that the vnode returned by vget() can't be recygled, I think this is true. If you mean that vget() can't return a clean vnode then this is false: vget() can sleep in vn_lock(), and it releases the v_interlock mutex before sleeping. While sleeping vclean() can VOP_RECLAIM() the vnode, even if v_usecount is 1. What is the practical difference of cleaned and recycled for the file system driver? From what I understand the vnode is not on the free list yet so it can't be reused for something else. If there is a race in vfs and XLOCK is not used properly, I think that should be investigated and fixed instead of patching file systems here and there. I don't know how XLOCK is supposed to be used (and I even less know how to change things without creating deadlocks). As I see it XLOCK is set while the vnode is being cleaned, not before or after cleaning it, so XLOCK is not going to avoid this race anyway. I asked for help on tech-kern about this but didn't get any reply ... -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Tue Sep 22 2009 at 21:04:14 +0200, Manuel Bouyer wrote: that's not an issue with the reference count. It's an issue with vclean() calling VOP_RECLAIM() even if the refcount is greater than 1, and vrelel() calling vclean() even if the refcount is greater than 1, when the file has been unlink(2)ed. There's a comment about this in vrelel(), look for variable recycle. Ah, ic, probably would've been easier if I read the PR first ;) So basically someone can vget the vnode (via fhtovp, since it's gone from the directory namespace) between VOP_INACTIVE and clean. Your fix doesn't really fix this problem, since depending on timings the inode might still be recycled after you check it's valid. Hmm, ok, so things which might happen: 1: VOP_REMOVE, vnode is locked, vrele is called 2: fhtovp before inode is reclaimed, blocks on vn_lock 1: VOP_INACTIVE releases vnlock, returns signal to recycle vnode 2: gets lock, does check that it's still the same inode 1: reclaims vnode 2: boom Since vget takes the interlock, a dirty cheap trick might be to check that the reference count is still one before trying to clean the vnode in vrelel(), otherwise punting and letting the next release-to-0 reclaim it? Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment?
Re: CVS commit: src/sys/ufs/ufs
On Tue, Sep 22, 2009 at 10:42:59PM +0300, Antti Kantee wrote: On Tue Sep 22 2009 at 21:04:14 +0200, Manuel Bouyer wrote: that's not an issue with the reference count. It's an issue with vclean() calling VOP_RECLAIM() even if the refcount is greater than 1, and vrelel() calling vclean() even if the refcount is greater than 1, when the file has been unlink(2)ed. There's a comment about this in vrelel(), look for variable recycle. Ah, ic, probably would've been easier if I read the PR first ;) So basically someone can vget the vnode (via fhtovp, since it's gone from the directory namespace) between VOP_INACTIVE and clean. Your fix doesn't really fix this problem, since depending on timings the inode might still be recycled after you check it's valid. I don't think so because at this point the vnode is locked. I think the race window is between vn_lock() releasing the interlock and getting the vn_lock. After that the reference count is high enouth to prevent vrelel() trying to clean it (vtryrele() at the start of vrelel will make it return, and we hold the interlock at this point). Hmm, ok, so things which might happen: 1: VOP_REMOVE, vnode is locked, vrele is called 2: fhtovp before inode is reclaimed, blocks on vn_lock It would fist block on the interlock at this point, I guess. 1: VOP_INACTIVE releases vnlock, returns signal to recycle vnode 2: gets lock, does check that it's still the same inode 1: reclaims vnode 2: boom Since vget takes the interlock, a dirty cheap trick might be to check that the reference count is still one before trying to clean the vnode in vrelel(), otherwise punting and letting the next release-to-0 reclaim it? So basically ignoring what VOP_INACTIVE() says. I think this would need another flag so that new vget() against this vnode can drop it early. Otherwise we could have a livelock with the NFS server always getting references to the vnode, preventing it from being recycled and blocking other threads waiting on the inode to reuse it. Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? that would be nice :) -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Tue Sep 22 2009 at 21:58:02 +0200, Manuel Bouyer wrote: [explanations] This is starting to sound sensible now. You obviously have invested quite a bit of thought in investigating the problem. Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? that would be nice :) You don't like my comments? ;)
Re: CVS commit: src/sys/ufs/ufs
On Tue, Sep 22, 2009 at 11:26:21PM +0300, Antti Kantee wrote: On Tue Sep 22 2009 at 21:58:02 +0200, Manuel Bouyer wrote: [explanations] This is starting to sound sensible now. You obviously have invested quite a bit of thought in investigating the problem. Yes, it was annoying enough :) Blah, I didn't even want to think about this migrane-inducer now. Maybe people who have recently worked on vnode reclaiming could instead be the ones to comment? that would be nice :) You don't like my comments? ;) Ho I do, and you certainly know more than I do in this area ... :) -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/ufs/ufs
On Sun Sep 20 2009 at 14:00:24 +, Manuel Bouyer wrote: Module Name: src Committed By: bouyer Date: Sun Sep 20 14:00:24 UTC 2009 Modified Files: src/sys/ufs/ufs: ufs_ihash.c Log Message: PR kern/41147: race between nfsd and local rm Note that the race also exists between 2 nfs client, one of them doing the rm. In ufs_ihashget(), vget() can return a vnode that has been vclean'ed because vget() can sleep. After vget returns, check that vp is still connected with ip, and that ip still points to the inode we want. This fix the NULL pointer dereference in ufs_fhtovp() I've been seeing on a NFS server. Um, hold the phone. The whole point of vget() is to provide race-free access to the weak vnode reference held by the file system. Are you saying this does not hold anymore?
