linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] setxattr bugs
@ 2013-02-03  4:30 Al Viro
  2013-02-03 13:59 ` Dave Kleikamp
  2013-02-05  2:14 ` Jeff Mahoney
  0 siblings, 2 replies; 4+ messages in thread
From: Al Viro @ 2013-02-03  4:30 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Jeff Mahoney, Dave Kleikamp

	* JFS, since 2005: setxattr(name, "system.posix_acl_access", NULL, 0, 0)
succeeds, creating an empty EA with "system.posix_acl_access" as name.
Validity checks should apply _after_
        if (value == NULL) {    /* empty EA, do not remove */
                value = "";
                value_len = 0;
        }
and not before it.
	* reiserfs, since 2009: setxattr(name, attr_name, NULL, 0, 0) is
treated as removexattr(name, attr_name), not as emptying given xattr.

	The question is, does either of those cross into "established
weirdness in ABI" or are they still at the "bugs to be fixed" stage?

	FWIW, I'm seriously tempted to stop passing NULL as the
third argument of ->setxattr(), essentially taking all those
if (!value) value = ""; pieces from individual ->setxattr() instances
to __vfs_setxattr_noperm() (all other callers of ->setxattr() never
pass NULL data or 0 size, so it's irrelevant for them).  Would fix
both jfs and reiserfs weirdness....

	Objections?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] setxattr bugs
  2013-02-03  4:30 [RFC] setxattr bugs Al Viro
@ 2013-02-03 13:59 ` Dave Kleikamp
  2013-02-05  2:14 ` Jeff Mahoney
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Kleikamp @ 2013-02-03 13:59 UTC (permalink / raw)
  To: Al Viro
  Cc: linux-kernel, Linus Torvalds, linux-fsdevel, Jeff Mahoney,
	Dave Kleikamp

On 02/02/2013 10:30 PM, Al Viro wrote:
> 	* JFS, since 2005: setxattr(name, "system.posix_acl_access", NULL, 0, 0)
> succeeds, creating an empty EA with "system.posix_acl_access" as name.
> Validity checks should apply _after_
>         if (value == NULL) {    /* empty EA, do not remove */
>                 value = "";
>                 value_len = 0;
>         }
> and not before it.

This is probably a hold-over from weird OS/2 behavior that we really
don't need to keep.

> 	* reiserfs, since 2009: setxattr(name, attr_name, NULL, 0, 0) is
> treated as removexattr(name, attr_name), not as emptying given xattr.
> 
> 	The question is, does either of those cross into "established
> weirdness in ABI" or are they still at the "bugs to be fixed" stage?
> 
> 	FWIW, I'm seriously tempted to stop passing NULL as the
> third argument of ->setxattr(), essentially taking all those
> if (!value) value = ""; pieces from individual ->setxattr() instances
> to __vfs_setxattr_noperm() (all other callers of ->setxattr() never
> pass NULL data or 0 size, so it's irrelevant for them).  Would fix
> both jfs and reiserfs weirdness....
> 
> 	Objections?

no objection from me. I can clean up the jfs code so that it no longer
saves empty xattrs.

Thanks,
Shaggy

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] setxattr bugs
  2013-02-03  4:30 [RFC] setxattr bugs Al Viro
  2013-02-03 13:59 ` Dave Kleikamp
@ 2013-02-05  2:14 ` Jeff Mahoney
  2013-02-05 17:14   ` Casey Schaufler
  1 sibling, 1 reply; 4+ messages in thread
From: Jeff Mahoney @ 2013-02-05  2:14 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel, Dave Kleikamp

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/2/13 11:30 PM, Al Viro wrote:
> * JFS, since 2005: setxattr(name, "system.posix_acl_access", NULL,
> 0, 0) succeeds, creating an empty EA with "system.posix_acl_access"
> as name. Validity checks should apply _after_ if (value == NULL) {
> /* empty EA, do not remove */ value = ""; value_len = 0; } and not
> before it. * reiserfs, since 2009: setxattr(name, attr_name, NULL,
> 0, 0) is treated as removexattr(name, attr_name), not as emptying
> given xattr.
> 
> The question is, does either of those cross into "established 
> weirdness in ABI" or are they still at the "bugs to be fixed"
> stage?

Since the behavior changed once already in 2009 I'd call it a bug.
That code was in the SLES kernel for a while before then and I still
haven't seen a bug report on it.

- -Jeff

> FWIW, I'm seriously tempted to stop passing NULL as the third
> argument of ->setxattr(), essentially taking all those if (!value)
> value = ""; pieces from individual ->setxattr() instances to
> __vfs_setxattr_noperm() (all other callers of ->setxattr() never 
> pass NULL data or 0 size, so it's irrelevant for them).  Would fix 
> both jfs and reiserfs weirdness....
> 
> Objections?




- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.18 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJREGrwAAoJEB57S2MheeWyHvMP/3kpy3Y4U0KNavnPaeL12LXe
RC6vIb/dPkoSemFiZ5om26aT70M7MdXJY2ZPCwgtlNpKV6aT0NFchtwiWos2lLLN
XndvFZ4M/kQLd9yDEmlcTDZn7p4fhU2Tn7FYrhPLRmOO3zP6fnUxLozSebOnGTO/
xEwV7Qtx7D4Au37khFW/hJvsAJE2Q3NrLgueIJLiTmFvSiOourZNmriNcB73MUeb
vYx5gc/bJexS2oFWeQqD6WiL8UQXg4XEKRk4inNVrJWpLV365w45Kpf2zBlvCQwQ
W8mdHcHoityOcQJtiXvnVDurUNpFwsthrhVquVgIopovlcvOjNtcpffH8YI9khP/
yol7+57ZDuVx2TY5DrEOa+TOTUrg5ghqagSSmOVDsOVeMngpdFNs8351QcX0IWBn
Xt8/eq46g/R7EHI3I1eYJHlMIie0hP1GDc66OP94hcKEWaHbPeKwkSTOlqYH++4h
ncSJcxHXWLUTGuV4b61whYTlJ2vBWwEvIteVaQmmXKaOTr41lajZBCWZDeUlzna8
XyJHE5FrcKDLzTNP1R7UNEj863fN0OUma1AKaT/6jNYMqFXOk39emTgZL5QfxP9X
uLWG1OVDf87uw5nYOKubNQiORpxl8iSIsQWvZeF9SvvmFA/JzpgZgtLlNqYa78Yv
oEq501m9BSEWVSGKxHcu
=G2cU
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC] setxattr bugs
  2013-02-05  2:14 ` Jeff Mahoney
@ 2013-02-05 17:14   ` Casey Schaufler
  0 siblings, 0 replies; 4+ messages in thread
From: Casey Schaufler @ 2013-02-05 17:14 UTC (permalink / raw)
  To: Jeff Mahoney
  Cc: Al Viro, linux-kernel, Linus Torvalds, linux-fsdevel,
	Dave Kleikamp, LSM, Casey Schaufler

On 2/4/2013 6:14 PM, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 2/2/13 11:30 PM, Al Viro wrote:
>> * JFS, since 2005: setxattr(name, "system.posix_acl_access", NULL,
>> 0, 0) succeeds, creating an empty EA with "system.posix_acl_access"
>> as name. Validity checks should apply _after_ if (value == NULL) {
>> /* empty EA, do not remove */ value = ""; value_len = 0; } and not
>> before it. * reiserfs, since 2009: setxattr(name, attr_name, NULL,
>> 0, 0) is treated as removexattr(name, attr_name), not as emptying
>> given xattr.
>>
>> The question is, does either of those cross into "established 
>> weirdness in ABI" or are they still at the "bugs to be fixed"
>> stage?
> Since the behavior changed once already in 2009 I'd call it a bug.
> That code was in the SLES kernel for a while before then and I still
> haven't seen a bug report on it.
>
> - -Jeff
>
>> FWIW, I'm seriously tempted to stop passing NULL as the third
>> argument of ->setxattr(), essentially taking all those if (!value)
>> value = ""; pieces from individual ->setxattr() instances to
>> __vfs_setxattr_noperm() (all other callers of ->setxattr() never 
>> pass NULL data or 0 size, so it's irrelevant for them).  Would fix 
>> both jfs and reiserfs weirdness....
>>
>> Objections?

None from me. I know of no one using either of those filesystems
in ways I care about. There's a chance the SELinux crowd may care,
so I'm adding the LSM list. We're the primary consumer of xattrs.

>
> - -- 
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.18 (Darwin)
> Comment: GPGTools - http://gpgtools.org
>
> iQIcBAEBAgAGBQJREGrwAAoJEB57S2MheeWyHvMP/3kpy3Y4U0KNavnPaeL12LXe
> RC6vIb/dPkoSemFiZ5om26aT70M7MdXJY2ZPCwgtlNpKV6aT0NFchtwiWos2lLLN
> XndvFZ4M/kQLd9yDEmlcTDZn7p4fhU2Tn7FYrhPLRmOO3zP6fnUxLozSebOnGTO/
> xEwV7Qtx7D4Au37khFW/hJvsAJE2Q3NrLgueIJLiTmFvSiOourZNmriNcB73MUeb
> vYx5gc/bJexS2oFWeQqD6WiL8UQXg4XEKRk4inNVrJWpLV365w45Kpf2zBlvCQwQ
> W8mdHcHoityOcQJtiXvnVDurUNpFwsthrhVquVgIopovlcvOjNtcpffH8YI9khP/
> yol7+57ZDuVx2TY5DrEOa+TOTUrg5ghqagSSmOVDsOVeMngpdFNs8351QcX0IWBn
> Xt8/eq46g/R7EHI3I1eYJHlMIie0hP1GDc66OP94hcKEWaHbPeKwkSTOlqYH++4h
> ncSJcxHXWLUTGuV4b61whYTlJ2vBWwEvIteVaQmmXKaOTr41lajZBCWZDeUlzna8
> XyJHE5FrcKDLzTNP1R7UNEj863fN0OUma1AKaT/6jNYMqFXOk39emTgZL5QfxP9X
> uLWG1OVDf87uw5nYOKubNQiORpxl8iSIsQWvZeF9SvvmFA/JzpgZgtLlNqYa78Yv
> oEq501m9BSEWVSGKxHcu
> =G2cU
> -----END PGP SIGNATURE-----
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-02-05 17:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-03  4:30 [RFC] setxattr bugs Al Viro
2013-02-03 13:59 ` Dave Kleikamp
2013-02-05  2:14 ` Jeff Mahoney
2013-02-05 17:14   ` Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).