Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
On 5/19/20 8:34 AM, Rodney W. Grimes wrote: > The EISDIR behavior is what your changing, independent of file system(s) > you have done so far. The fact the behavior is now different between > UFS and ZFS is sic, IMHO. Oh please. NFS already fails with ESIDIR and has failed to read() directories for 30 years: https://svnweb.freebsd.org/csrg?view=revision=41905 We haven't had riots in the streets over that. If you actually looked at what ZFS was returning for read(), it was useless. -- John Baldwin ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
On Tue, May 19, 2020 at 10:34 AM Rodney W. Grimes wrote: > > > On Tue, May 19, 2020 at 10:27 AM Rodney W. Grimes > > wrote: > > > > > > > On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes > > > > wrote: > > > > > > > > > > > Author: kevans > > > > > > Date: Tue May 19 02:41:05 2020 > > > > > > New Revision: 361238 > > > > > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > > > > > > > > > Log: > > > > > > zfs: reject read(2) of a dirfd with EISDIR > > > > > > > > > > > > This is independent of the recently-discussed global change, > > > > > > which is still > > > > > > in review/discussion stage. > > > > > > > > > > > > This is effectively a measure for consistency in the ZFS world, > > > > > > where > > > > > > FreeBSD was the only platform (as far as I could find) that > > > > > > allowed this. > > > > > > What ZFS exposes is decidedly not useful for any real purposes, to > > > > > > paraphrase (hopefully faithfully) jhb's findings when exploring > > > > > > this: > > > > > > > > > > > > The size of a directory in ZFS is the number of directory entries > > > > > > within. > > > > > > When reading a directory, you would instead get the leading part > > > > > > of its raw > > > > > > contents; the amount you get being dictated by the "size," i.e. > > > > > > number of > > > > > > directory entries. There's decidedly (luckily) no stack > > > > > > disclosure happening > > > > > > here, though the behavior is bizarre and almost certainly a > > > > > > historical > > > > > > accident. > > > > > > > > > > > > This change has already been upstreamed to OpenZFS. > > > > > > > > > > Until the grep -d skip issue is addressed I object to this change as > > > > > it is going to cause people who do grep with wildcards to see lots > > > > > of errors that before where pretty much either silent (no match > > > > > occured) > > > > > or spit out a "binary file foo matches." > > > > > > > > > > > > > That seems preferable to grepping random bytes that don't particularly > > > > contain any strings? They'd never see "binary file foo matches" in > > > > this case. > > > > > > The difference is you rarely get a hit, and now your gauranteed to > > > get a hit on every single directory making grep * very noisy, where > > > it was often silent or nearly silent before. > > > > > > > As you noted in the review for the larger change, -d skip is a good > > option for the people that don't like this. It probably makes sense as > > a default, but then we'd be diverging from the other popular grep that > > defaults to -d read and spews out EISDIR more often than not. > > Yet another thing I hate about Linux, thank you for adding it to FreeBSD :-) > > > > > > > > > This isn't exactly divergent from the behavior they'd see with ZFS > > > > anywhere else. > > > > > > It is extremly divergent from 42 years of behavior. > > > > > > > I don't think ZFS has been implemented on FreeBSD for 42 years, and I > > don't find this grep argument compelling enough to restore peoples' > > ability to read the raw znode of a directory. > > The EISDIR behavior is what your changing, independent of file system(s) > you have done so far. The fact the behavior is now different between > UFS and ZFS is sic, IMHO. EISDIR in read(2) denotes that a filesystem does not support reading a directory, this isn't a new kind of error. In particular, ZFS traditionally does NOT support reading a directory like this. The behavior between UFS and ZFS should have always been different, this is correction of a historical *accident*. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
> On Tue, May 19, 2020 at 10:27 AM Rodney W. Grimes > wrote: > > > > > On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes > > > wrote: > > > > > > > > > Author: kevans > > > > > Date: Tue May 19 02:41:05 2020 > > > > > New Revision: 361238 > > > > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > > > > > > > Log: > > > > > zfs: reject read(2) of a dirfd with EISDIR > > > > > > > > > > This is independent of the recently-discussed global change, which > > > > > is still > > > > > in review/discussion stage. > > > > > > > > > > This is effectively a measure for consistency in the ZFS world, > > > > > where > > > > > FreeBSD was the only platform (as far as I could find) that allowed > > > > > this. > > > > > What ZFS exposes is decidedly not useful for any real purposes, to > > > > > paraphrase (hopefully faithfully) jhb's findings when exploring > > > > > this: > > > > > > > > > > The size of a directory in ZFS is the number of directory entries > > > > > within. > > > > > When reading a directory, you would instead get the leading part of > > > > > its raw > > > > > contents; the amount you get being dictated by the "size," i.e. > > > > > number of > > > > > directory entries. There's decidedly (luckily) no stack disclosure > > > > > happening > > > > > here, though the behavior is bizarre and almost certainly a > > > > > historical > > > > > accident. > > > > > > > > > > This change has already been upstreamed to OpenZFS. > > > > > > > > Until the grep -d skip issue is addressed I object to this change as > > > > it is going to cause people who do grep with wildcards to see lots > > > > of errors that before where pretty much either silent (no match occured) > > > > or spit out a "binary file foo matches." > > > > > > > > > > That seems preferable to grepping random bytes that don't particularly > > > contain any strings? They'd never see "binary file foo matches" in > > > this case. > > > > The difference is you rarely get a hit, and now your gauranteed to > > get a hit on every single directory making grep * very noisy, where > > it was often silent or nearly silent before. > > > > As you noted in the review for the larger change, -d skip is a good > option for the people that don't like this. It probably makes sense as > a default, but then we'd be diverging from the other popular grep that > defaults to -d read and spews out EISDIR more often than not. Yet another thing I hate about Linux, thank you for adding it to FreeBSD :-) > > > > > > This isn't exactly divergent from the behavior they'd see with ZFS > > > anywhere else. > > > > It is extremly divergent from 42 years of behavior. > > > > I don't think ZFS has been implemented on FreeBSD for 42 years, and I > don't find this grep argument compelling enough to restore peoples' > ability to read the raw znode of a directory. The EISDIR behavior is what your changing, independent of file system(s) you have done so far. The fact the behavior is now different between UFS and ZFS is sic, IMHO. -- Rod Grimes rgri...@freebsd.org ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
On Tue, May 19, 2020 at 10:27 AM Rodney W. Grimes wrote: > > > On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes > > wrote: > > > > > > > Author: kevans > > > > Date: Tue May 19 02:41:05 2020 > > > > New Revision: 361238 > > > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > > > > > Log: > > > > zfs: reject read(2) of a dirfd with EISDIR > > > > > > > > This is independent of the recently-discussed global change, which is > > > > still > > > > in review/discussion stage. > > > > > > > > This is effectively a measure for consistency in the ZFS world, where > > > > FreeBSD was the only platform (as far as I could find) that allowed > > > > this. > > > > What ZFS exposes is decidedly not useful for any real purposes, to > > > > paraphrase (hopefully faithfully) jhb's findings when exploring this: > > > > > > > > The size of a directory in ZFS is the number of directory entries > > > > within. > > > > When reading a directory, you would instead get the leading part of > > > > its raw > > > > contents; the amount you get being dictated by the "size," i.e. > > > > number of > > > > directory entries. There's decidedly (luckily) no stack disclosure > > > > happening > > > > here, though the behavior is bizarre and almost certainly a historical > > > > accident. > > > > > > > > This change has already been upstreamed to OpenZFS. > > > > > > Until the grep -d skip issue is addressed I object to this change as > > > it is going to cause people who do grep with wildcards to see lots > > > of errors that before where pretty much either silent (no match occured) > > > or spit out a "binary file foo matches." > > > > > > > That seems preferable to grepping random bytes that don't particularly > > contain any strings? They'd never see "binary file foo matches" in > > this case. > > The difference is you rarely get a hit, and now your gauranteed to > get a hit on every single directory making grep * very noisy, where > it was often silent or nearly silent before. > As you noted in the review for the larger change, -d skip is a good option for the people that don't like this. It probably makes sense as a default, but then we'd be diverging from the other popular grep that defaults to -d read and spews out EISDIR more often than not. > > > > This isn't exactly divergent from the behavior they'd see with ZFS > > anywhere else. > > It is extremly divergent from 42 years of behavior. > I don't think ZFS has been implemented on FreeBSD for 42 years, and I don't find this grep argument compelling enough to restore peoples' ability to read the raw znode of a directory. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
> > On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes > > wrote: > > > > > > > Author: kevans > > > > Date: Tue May 19 02:41:05 2020 > > > > New Revision: 361238 > > > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > > > > > Log: > > > > zfs: reject read(2) of a dirfd with EISDIR > > > > > > > > This is independent of the recently-discussed global change, which is > > > > still > > > > in review/discussion stage. > > > > > > > > This is effectively a measure for consistency in the ZFS world, where > > > > FreeBSD was the only platform (as far as I could find) that allowed > > > > this. > > > > What ZFS exposes is decidedly not useful for any real purposes, to > > > > paraphrase (hopefully faithfully) jhb's findings when exploring this: > > > > > > > > The size of a directory in ZFS is the number of directory entries > > > > within. > > > > When reading a directory, you would instead get the leading part of > > > > its raw > > > > contents; the amount you get being dictated by the "size," i.e. > > > > number of > > > > directory entries. There's decidedly (luckily) no stack disclosure > > > > happening > > > > here, though the behavior is bizarre and almost certainly a historical > > > > accident. > > > > > > > > This change has already been upstreamed to OpenZFS. > > > > > > Until the grep -d skip issue is addressed I object to this change as > > > it is going to cause people who do grep with wildcards to see lots > > > of errors that before where pretty much either silent (no match occured) > > > or spit out a "binary file foo matches." > > > > > > > That seems preferable to grepping random bytes that don't particularly > > contain any strings? They'd never see "binary file foo matches" in > > this case. > > The difference is you rarely get a hit, and now your gauranteed to > get a hit on every single directory making grep * very noisy, where > it was often silent or nearly silent before. Please, go try this and see if you can see why I am asserting what I am: (on one of your patched systems) cd /etc grep foo * grep -d skip foo * #This makes it closer to old behavior, not exact as binary matches shall be skipped. The first command isg going to return an error for every directory, the second command, closer to historical behavior, is going to be nearly silent. You could run the second command on a pre patched system, ufs or zfs wont matter much. > > This isn't exactly divergent from the behavior they'd see with ZFS > > anywhere else. > > It is extremly divergent from 42 years of behavior. > > > Thanks, > > > > Kyle Evans > > > > -- > Rod Grimes rgri...@freebsd.org > -- Rod Grimes rgri...@freebsd.org ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
> On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes > wrote: > > > > > Author: kevans > > > Date: Tue May 19 02:41:05 2020 > > > New Revision: 361238 > > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > > > Log: > > > zfs: reject read(2) of a dirfd with EISDIR > > > > > > This is independent of the recently-discussed global change, which is > > > still > > > in review/discussion stage. > > > > > > This is effectively a measure for consistency in the ZFS world, where > > > FreeBSD was the only platform (as far as I could find) that allowed > > > this. > > > What ZFS exposes is decidedly not useful for any real purposes, to > > > paraphrase (hopefully faithfully) jhb's findings when exploring this: > > > > > > The size of a directory in ZFS is the number of directory entries > > > within. > > > When reading a directory, you would instead get the leading part of its > > > raw > > > contents; the amount you get being dictated by the "size," i.e. number > > > of > > > directory entries. There's decidedly (luckily) no stack disclosure > > > happening > > > here, though the behavior is bizarre and almost certainly a historical > > > accident. > > > > > > This change has already been upstreamed to OpenZFS. > > > > Until the grep -d skip issue is addressed I object to this change as > > it is going to cause people who do grep with wildcards to see lots > > of errors that before where pretty much either silent (no match occured) > > or spit out a "binary file foo matches." > > > > That seems preferable to grepping random bytes that don't particularly > contain any strings? They'd never see "binary file foo matches" in > this case. The difference is you rarely get a hit, and now your gauranteed to get a hit on every single directory making grep * very noisy, where it was often silent or nearly silent before. > > This isn't exactly divergent from the behavior they'd see with ZFS > anywhere else. It is extremly divergent from 42 years of behavior. > Thanks, > > Kyle Evans > -- Rod Grimes rgri...@freebsd.org ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
On Tue, May 19, 2020 at 10:23 AM Rodney W. Grimes wrote: > > > Author: kevans > > Date: Tue May 19 02:41:05 2020 > > New Revision: 361238 > > URL: https://svnweb.freebsd.org/changeset/base/361238 > > > > Log: > > zfs: reject read(2) of a dirfd with EISDIR > > > > This is independent of the recently-discussed global change, which is > > still > > in review/discussion stage. > > > > This is effectively a measure for consistency in the ZFS world, where > > FreeBSD was the only platform (as far as I could find) that allowed this. > > What ZFS exposes is decidedly not useful for any real purposes, to > > paraphrase (hopefully faithfully) jhb's findings when exploring this: > > > > The size of a directory in ZFS is the number of directory entries within. > > When reading a directory, you would instead get the leading part of its > > raw > > contents; the amount you get being dictated by the "size," i.e. number of > > directory entries. There's decidedly (luckily) no stack disclosure > > happening > > here, though the behavior is bizarre and almost certainly a historical > > accident. > > > > This change has already been upstreamed to OpenZFS. > > Until the grep -d skip issue is addressed I object to this change as > it is going to cause people who do grep with wildcards to see lots > of errors that before where pretty much either silent (no match occured) > or spit out a "binary file foo matches." > That seems preferable to grepping random bytes that don't particularly contain any strings? They'd never see "binary file foo matches" in this case. This isn't exactly divergent from the behavior they'd see with ZFS anywhere else. Thanks, Kyle Evans ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
> Author: kevans
> Date: Tue May 19 02:41:05 2020
> New Revision: 361238
> URL: https://svnweb.freebsd.org/changeset/base/361238
>
> Log:
> zfs: reject read(2) of a dirfd with EISDIR
>
> This is independent of the recently-discussed global change, which is still
> in review/discussion stage.
>
> This is effectively a measure for consistency in the ZFS world, where
> FreeBSD was the only platform (as far as I could find) that allowed this.
> What ZFS exposes is decidedly not useful for any real purposes, to
> paraphrase (hopefully faithfully) jhb's findings when exploring this:
>
> The size of a directory in ZFS is the number of directory entries within.
> When reading a directory, you would instead get the leading part of its raw
> contents; the amount you get being dictated by the "size," i.e. number of
> directory entries. There's decidedly (luckily) no stack disclosure happening
> here, though the behavior is bizarre and almost certainly a historical
> accident.
>
> This change has already been upstreamed to OpenZFS.
Until the grep -d skip issue is addressed I object to this change as
it is going to cause people who do grep with wildcards to see lots
of errors that before where pretty much either silent (no match occured)
or spit out a "binary file foo matches."
>
> MFC after: 1 week
Please no.
> 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 Tue May
> 19 02:07:08 2020(r361237)
> +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Tue May
> 19 02:41:05 2020(r361238)
> @@ -646,6 +646,12 @@ zfs_read(vnode_t *vp, uio_t *uio, int ioflag, cred_t *
> ZFS_ENTER(zfsvfs);
> ZFS_VERIFY_ZP(zp);
>
> + /* We don't copy out anything useful for directories. */
> + if (vp->v_type == VDIR) {
> + ZFS_EXIT(zfsvfs);
> + return (SET_ERROR(EISDIR));
> + }
> +
> if (zp->z_pflags & ZFS_AV_QUARANTINED) {
> ZFS_EXIT(zfsvfs);
> return (SET_ERROR(EACCES));
>
--
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r361238 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Author: kevans
Date: Tue May 19 02:41:05 2020
New Revision: 361238
URL: https://svnweb.freebsd.org/changeset/base/361238
Log:
zfs: reject read(2) of a dirfd with EISDIR
This is independent of the recently-discussed global change, which is still
in review/discussion stage.
This is effectively a measure for consistency in the ZFS world, where
FreeBSD was the only platform (as far as I could find) that allowed this.
What ZFS exposes is decidedly not useful for any real purposes, to
paraphrase (hopefully faithfully) jhb's findings when exploring this:
The size of a directory in ZFS is the number of directory entries within.
When reading a directory, you would instead get the leading part of its raw
contents; the amount you get being dictated by the "size," i.e. number of
directory entries. There's decidedly (luckily) no stack disclosure happening
here, though the behavior is bizarre and almost certainly a historical
accident.
This change has already been upstreamed to OpenZFS.
MFC after:1 week
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 Tue May
19 02:07:08 2020(r361237)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c Tue May
19 02:41:05 2020(r361238)
@@ -646,6 +646,12 @@ zfs_read(vnode_t *vp, uio_t *uio, int ioflag, cred_t *
ZFS_ENTER(zfsvfs);
ZFS_VERIFY_ZP(zp);
+ /* We don't copy out anything useful for directories. */
+ if (vp->v_type == VDIR) {
+ ZFS_EXIT(zfsvfs);
+ return (SET_ERROR(EISDIR));
+ }
+
if (zp->z_pflags & ZFS_AV_QUARANTINED) {
ZFS_EXIT(zfsvfs);
return (SET_ERROR(EACCES));
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
