Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 17 Jan 2011 16:32:18 -0800
From:      Garrett Cooper <yanegomi@gmail.com>
To:        fs@freebsd.org
Subject:   Inconsistency and bug with *chflags functions
Message-ID:  <AANLkTintj_eM=aRF4pO8PcT8VXvVJ=Booxag%2BwBg3gaX@mail.gmail.com>

next in thread | raw e-mail | index | archive | help
--000e0cdfd8f8794ebc049a140808
Content-Type: text/plain; charset=ISO-8859-1

Hi FS folks,

    Originally I did this to determine why lchflags had an int
argument and chflags/fchflags had unsigned long arguments, and I ran
into an interesting mini-project (by accident) after I found a few
bugs lurking in the vfs_syscalls.c layer of the kernel.

    So here's the deal...

Problems:
1. chflags/fchflags claim that the flags argument is an unsigned long,
whereas lchflags claims it's an int. This is inconsistent.
2. The kernel interfaces are all implicitly downcasting the flags
argument to int.
3. There's a sizing/signedness discrepancy in a few other structures
with fixed-width data types (uint32_t).

Solution:
1. I opted to convert fflags_t to uint32_t and convert all references
which did direct conversions to and from st_flags to fflags_t to avoid
32-bit / 64-bit biarch issues. I downgraded it to uint32_t as we're no
where near the limit for the number of usable flags to *chflags(2).
2. *chflags now uses fflags_t instead of int/unsigned long.
3. c_file_flags in dumprestore.h was changed to be in sync with st_flags.
4. di_flags in ufs/ufs/dinode.h was changed to be in sync with st_flags.

Compatibility:
1. NetBSD uses unsigned long in their chflags calls (both in kernel
and userland) so they're more consistent than we are by not having
mixed flags calling convention like us, but uses uint32_t in their
data structures (like we do), so they have a 32-bit/64-bit biarch bug
(again like we do).
2. OpenBSD is using unsigned int, so I assume that their kernel layer
is also using unsigned int (I am basing this purely off the manpage as
I haven't looked at their sources, but I could be wrong).

    I'm running make universe just to see if it will barf in the
build, but would someone please review this change (I made the change
as minimal as possible for ease of review) and provide feedback?
Thanks,
-Garrett

PS Please CC me on all replies as I'm not subscribed to the list.

--000e0cdfd8f8794ebc049a140808
Content-Type: text/x-patch; charset=US-ASCII;
	name="chflags-to-fflags_t-fix.patch"
Content-Disposition: attachment; filename="chflags-to-fflags_t-fix.patch"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_gj22gzh91

SW5kZXg6IGluY2x1ZGUvcHJvdG9jb2xzL2R1bXByZXN0b3JlLmgKPT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gaW5j
bHVkZS9wcm90b2NvbHMvZHVtcHJlc3RvcmUuaAkocmV2aXNpb24gMjE3MzYyKQorKysgaW5jbHVk
ZS9wcm90b2NvbHMvZHVtcHJlc3RvcmUuaAkod29ya2luZyBjb3B5KQpAQCAtOTUsNyArOTUsNyBA
QAogCQlpbnQ2NF90CWNfbXRpbWU7CSAgICAvKiBsYXN0IG1vZGlmaWVkIHRpbWUsIHNlY29uZHMg
Ki8KIAkJaW50MzJfdAljX2V4dHNpemU7CSAgICAvKiBleHRlcm5hbCBhdHRyaWJ1dGUgc2l6ZSAq
LwogCQlpbnQzMl90CWNfc3BhcmU0WzZdOwkgICAgLyogb2xkIGJsb2NrIHBvaW50ZXJzICovCi0J
CXVfaW50MzJfdCBjX2ZpbGVfZmxhZ3M7CSAgICAvKiBzdGF0dXMgZmxhZ3MgKGNoZmxhZ3MpICov
CisJCWZmbGFnc190IGNfZmlsZV9mbGFnczsJICAgIC8qIHN0YXR1cyBmbGFncyAoY2hmbGFncykg
Ki8KIAkJaW50MzJfdAljX3NwYXJlNVsyXTsJICAgIC8qIG9sZCBibG9ja3MsIGdlbmVyYXRpb24g
bnVtYmVyICovCiAJCXVfaW50MzJfdCBjX3VpZDsJICAgIC8qIGZpbGUgb3duZXIgKi8KIAkJdV9p
bnQzMl90IGNfZ2lkOwkgICAgLyogZmlsZSBncm91cCAqLwpJbmRleDogbGliL2xpYmMvc3lzL2No
ZmxhZ3MuMgo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09Ci0tLSBsaWIvbGliYy9zeXMvY2hmbGFncy4yCShyZXZpc2lvbiAy
MTczNjIpCisrKyBsaWIvbGliYy9zeXMvY2hmbGFncy4yCSh3b3JraW5nIGNvcHkpCkBAIC00Miwx
MSArNDIsMTEgQEAKIC5JbiBzeXMvc3RhdC5oCiAuSW4gdW5pc3RkLmgKIC5GdCBpbnQKLS5GbiBj
aGZsYWdzICJjb25zdCBjaGFyICpwYXRoIiAidV9sb25nIGZsYWdzIgorLkZuIGNoZmxhZ3MgImNv
bnN0IGNoYXIgKnBhdGgiICJmZmxhZ3NfdCBmbGFncyIKIC5GdCBpbnQKLS5GbiBsY2hmbGFncyAi
Y29uc3QgY2hhciAqcGF0aCIgImludCBmbGFncyIKKy5GbiBsY2hmbGFncyAiY29uc3QgY2hhciAq
cGF0aCIgImZmbGFnc190IGZsYWdzIgogLkZ0IGludAotLkZuIGZjaGZsYWdzICJpbnQgZmQiICJ1
X2xvbmcgZmxhZ3MiCisuRm4gZmNoZmxhZ3MgImludCBmZCIgImZmbGFnc190IGZsYWdzIgogLlNo
IERFU0NSSVBUSU9OCiBUaGUgZmlsZSB3aG9zZSBuYW1lCiBpcyBnaXZlbiBieQpJbmRleDogc2Jp
bi9yZXN0b3JlL3RhcGUuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBzYmluL3Jlc3RvcmUvdGFwZS5jCShyZXZp
c2lvbiAyMTczNjIpCisrKyBzYmluL3Jlc3RvcmUvdGFwZS5jCSh3b3JraW5nIGNvcHkpCkBAIC01
NTgsNyArNTU4LDcgQEAKIGludAogZXh0cmFjdGZpbGUoY2hhciAqbmFtZSkKIHsKLQlpbnQgZmxh
Z3M7CisJZmZsYWdzX3QgZmxhZ3M7CiAJdWlkX3QgdWlkOwogCWdpZF90IGdpZDsKIAltb2RlX3Qg
bW9kZTsKSW5kZXg6IHN5cy9rZXJuL3Zmc19zeXNjYWxscy5jCj09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9r
ZXJuL3Zmc19zeXNjYWxscy5jCShyZXZpc2lvbiAyMTczNjIpCisrKyBzeXMva2Vybi92ZnNfc3lz
Y2FsbHMuYwkod29ya2luZyBjb3B5KQpAQCAtOTYsNyArOTYsNyBAQAogc3RhdGljIGludCBnZXR1
dGltZXMoY29uc3Qgc3RydWN0IHRpbWV2YWwgKiwgZW51bSB1aW9fc2VnLCBzdHJ1Y3QgdGltZXNw
ZWMgKik7CiBzdGF0aWMgaW50IHNldGZvd24oc3RydWN0IHRocmVhZCAqdGQsIHN0cnVjdCB2bm9k
ZSAqLCB1aWRfdCwgZ2lkX3QpOwogc3RhdGljIGludCBzZXRmbW9kZShzdHJ1Y3QgdGhyZWFkICp0
ZCwgc3RydWN0IHZub2RlICosIGludCk7Ci1zdGF0aWMgaW50IHNldGZmbGFncyhzdHJ1Y3QgdGhy
ZWFkICp0ZCwgc3RydWN0IHZub2RlICosIGludCk7CitzdGF0aWMgaW50IHNldGZmbGFncyhzdHJ1
Y3QgdGhyZWFkICp0ZCwgc3RydWN0IHZub2RlICosIGZmbGFnc190KTsKIHN0YXRpYyBpbnQgc2V0
dXRpbWVzKHN0cnVjdCB0aHJlYWQgKnRkLCBzdHJ1Y3Qgdm5vZGUgKiwKICAgICBjb25zdCBzdHJ1
Y3QgdGltZXNwZWMgKiwgaW50LCBpbnQpOwogc3RhdGljIGludCB2bl9hY2Nlc3Moc3RydWN0IHZu
b2RlICp2cCwgaW50IHVzZXJfZmxhZ3MsIHN0cnVjdCB1Y3JlZCAqY3JlZCwKQEAgLTI2NTcsNyAr
MjY1Nyw3IEBACiBzZXRmZmxhZ3ModGQsIHZwLCBmbGFncykKIAlzdHJ1Y3QgdGhyZWFkICp0ZDsK
IAlzdHJ1Y3Qgdm5vZGUgKnZwOwotCWludCBmbGFnczsKKwlmZmxhZ3NfdCBmbGFnczsKIHsKIAlp
bnQgZXJyb3I7CiAJc3RydWN0IG1vdW50ICptcDsKQEAgLTI2OTUsOCArMjY5NSw4IEBACiAgKi8K
ICNpZm5kZWYgX1NZU19TWVNQUk9UT19IXwogc3RydWN0IGNoZmxhZ3NfYXJncyB7Ci0JY2hhcgkq
cGF0aDsKLQlpbnQJZmxhZ3M7CisJY2hhcgkJKnBhdGg7CisJZmZsYWdzX3QJZmxhZ3M7CiB9Owog
I2VuZGlmCiBpbnQKQEAgLTI3MDQsNyArMjcwNCw3IEBACiAJc3RydWN0IHRocmVhZCAqdGQ7CiAJ
cmVnaXN0ZXIgc3RydWN0IGNoZmxhZ3NfYXJncyAvKiB7CiAJCWNoYXIgKnBhdGg7Ci0JCWludCBm
bGFnczsKKwkJZmZsYWdzX3QgZmxhZ3M7CiAJfSAqLyAqdWFwOwogewogCWludCBlcnJvcjsKQEAg
LTI3MzIsNyArMjczMiw3IEBACiAJc3RydWN0IHRocmVhZCAqdGQ7CiAJcmVnaXN0ZXIgc3RydWN0
IGxjaGZsYWdzX2FyZ3MgLyogewogCQljaGFyICpwYXRoOwotCQlpbnQgZmxhZ3M7CisJCWZmbGFn
c190IGZsYWdzOwogCX0gKi8gKnVhcDsKIHsKIAlpbnQgZXJyb3I7CkBAIC0yNzU3LDggKzI3NTcs
OCBAQAogICovCiAjaWZuZGVmIF9TWVNfU1lTUFJPVE9fSF8KIHN0cnVjdCBmY2hmbGFnc19hcmdz
IHsKLQlpbnQJZmQ7Ci0JaW50CWZsYWdzOworCWludAkJZmQ7CisJZmZsYWdzX3QJZmxhZ3M7CiB9
OwogI2VuZGlmCiBpbnQKQEAgLTI3NjYsNyArMjc2Niw3IEBACiAJc3RydWN0IHRocmVhZCAqdGQ7
CiAJcmVnaXN0ZXIgc3RydWN0IGZjaGZsYWdzX2FyZ3MgLyogewogCQlpbnQgZmQ7Ci0JCWludCBm
bGFnczsKKwkJZmZsYWdzX3QgZmxhZ3M7CiAJfSAqLyAqdWFwOwogewogCXN0cnVjdCBmaWxlICpm
cDsKSW5kZXg6IHN5cy9zeXMvc3RhdC5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9zeXMvc3RhdC5oCShy
ZXZpc2lvbiAyMTczNjIpCisrKyBzeXMvc3lzL3N0YXQuaAkod29ya2luZyBjb3B5KQpAQCAtMjky
LDExICsyOTIsMTEgQEAKICNpZm5kZWYgX0tFUk5FTAogX19CRUdJTl9ERUNMUwogI2lmIF9fQlNE
X1ZJU0lCTEUKLWludAljaGZsYWdzKGNvbnN0IGNoYXIgKiwgdW5zaWduZWQgbG9uZyk7CitpbnQJ
Y2hmbGFncyhjb25zdCBjaGFyICosIGZmbGFnc190KTsKICNlbmRpZgogaW50CWNobW9kKGNvbnN0
IGNoYXIgKiwgbW9kZV90KTsKICNpZiBfX0JTRF9WSVNJQkxFCi1pbnQJZmNoZmxhZ3MoaW50LCB1
bnNpZ25lZCBsb25nKTsKK2ludAlmY2hmbGFncyhpbnQsIGZmbGFnc190KTsKICNlbmRpZgogI2lm
IF9fUE9TSVhfVklTSUJMRSA+PSAyMDAxMTIKIGludAlmY2htb2QoaW50LCBtb2RlX3QpOwpAQCAt
MzA2LDcgKzMwNiw3IEBACiAjZW5kaWYKIGludAlmc3RhdChpbnQsIHN0cnVjdCBzdGF0ICopOwog
I2lmIF9fQlNEX1ZJU0lCTEUKLWludAlsY2hmbGFncyhjb25zdCBjaGFyICosIGludCk7CitpbnQJ
bGNoZmxhZ3MoY29uc3QgY2hhciAqLCBmZmxhZ3NfdCk7CiBpbnQJbGNobW9kKGNvbnN0IGNoYXIg
KiwgbW9kZV90KTsKICNlbmRpZgogI2lmIF9fUE9TSVhfVklTSUJMRSA+PSAyMDAxMTIKSW5kZXg6
IHN5cy91ZnMvdWZzL2Rpbm9kZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy91ZnMvdWZzL2Rpbm9kZS5o
CShyZXZpc2lvbiAyMTczNjIpCisrKyBzeXMvdWZzL3Vmcy9kaW5vZGUuaAkod29ya2luZyBjb3B5
KQpAQCAtMTQwLDcgKzE0MCw3IEBACiAJaW50MzJfdAkJZGlfYmlydGhuc2VjOwkvKiAgNzY6IElu
b2RlIGNyZWF0aW9uIHRpbWUuICovCiAJaW50MzJfdAkJZGlfZ2VuOwkJLyogIDgwOiBHZW5lcmF0
aW9uIG51bWJlci4gKi8KIAl1X2ludDMyX3QJZGlfa2VybmZsYWdzOwkvKiAgODQ6IEtlcm5lbCBm
bGFncy4gKi8KLQl1X2ludDMyX3QJZGlfZmxhZ3M7CS8qICA4ODogU3RhdHVzIGZsYWdzIChjaGZs
YWdzKS4gKi8KKwlmZmxhZ3NfdAlkaV9mbGFnczsJLyogIDg4OiBTdGF0dXMgZmxhZ3MgKGNoZmxh
Z3MpLiAqLwogCWludDMyX3QJCWRpX2V4dHNpemU7CS8qICA5MjogRXh0ZXJuYWwgYXR0cmlidXRl
cyBibG9jay4gKi8KIAl1ZnMyX2RhZGRyX3QJZGlfZXh0YltOWEFERFJdOy8qICA5NjogRXh0ZXJu
YWwgYXR0cmlidXRlcyBibG9jay4gKi8KIAl1ZnMyX2RhZGRyX3QJZGlfZGJbTkRBRERSXTsJLyog
MTEyOiBEaXJlY3QgZGlzayBibG9ja3MuICovCkluZGV4OiB0b29scy9yZWdyZXNzaW9uL3BqZGZz
dGVzdC9wamRmc3Rlc3QuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSB0b29scy9yZWdyZXNzaW9uL3BqZGZzdGVz
dC9wamRmc3Rlc3QuYwkocmV2aXNpb24gMjE3MzYyKQorKysgdG9vbHMvcmVncmVzc2lvbi9wamRm
c3Rlc3QvcGpkZnN0ZXN0LmMJKHdvcmtpbmcgY29weSkKQEAgLTU3OCwxMiArNTc3LDE0IEBACiAJ
CWJyZWFrOwogI2lmZGVmIEhBU19DSEZMQUdTCiAJY2FzZSBBQ1RJT05fQ0hGTEFHUzoKLQkJcnZh
bCA9IGNoZmxhZ3MoU1RSKDApLCAodW5zaWduZWQgbG9uZylzdHIyZmxhZ3MoY2hmbGFnc19mbGFn
cywgU1RSKDEpKSk7CisJCXJ2YWwgPSBjaGZsYWdzKFNUUigwKSwKKwkJICAgIChmZmxhZ3NfdClz
dHIyZmxhZ3MoY2hmbGFnc19mbGFncywgU1RSKDEpKSk7CiAJCWJyZWFrOwogI2VuZGlmCiAjaWZk
ZWYgSEFTX0xDSEZMQUdTCiAJY2FzZSBBQ1RJT05fTENIRkxBR1M6Ci0JCXJ2YWwgPSBsY2hmbGFn
cyhTVFIoMCksIChpbnQpc3RyMmZsYWdzKGNoZmxhZ3NfZmxhZ3MsIFNUUigxKSkpOworCQlydmFs
ID0gbGNoZmxhZ3MoU1RSKDApLAorCQkgICAgKGZmbGFnc190KXN0cjJmbGFncyhjaGZsYWdzX2Zs
YWdzLCBTVFIoMSkpKTsKIAkJYnJlYWs7CiAjZW5kaWYKIAljYXNlIEFDVElPTl9UUlVOQ0FURToK
--000e0cdfd8f8794ebc049a140808--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTintj_eM=aRF4pO8PcT8VXvVJ=Booxag%2BwBg3gaX>