Date: Thu, 10 Feb 2011 09:36:34 -0700 From: Shawn Webb <lattera@gmail.com> To: Tim Kientzle <tim@kientzle.com> Cc: FreeBSD-current <freebsd-current@freebsd.org> Subject: Re: setfacl Recursive Functionality Message-ID: <AANLkTik4B1O=xx1MmjbdMVO6pwR=MaBjVaWU_bi4Ry8c@mail.gmail.com> In-Reply-To: <AANLkTim9anqAqSLH=5OMi_dTyAsqY2xCL37KM8MmrZdR@mail.gmail.com> References: <AANLkTi=%2BWtmRz07m=Cg7hbXJGw7eWRHC1ASGeufTSLBB@mail.gmail.com> <80373F51-25C7-48A0-8920-3444A98D857F@kientzle.com> <AANLkTin2XR9Ra4LK-rgh94XaAUB6jY_sg0n51GdbgnQ2@mail.gmail.com> <AANLkTim9anqAqSLH=5OMi_dTyAsqY2xCL37KM8MmrZdR@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--0016e659f5f45082b9049bf02fe6 Content-Type: text/plain; charset=ISO-8859-1 Attached is a new patch using the acl(3) API for removing invalid inheritance. Unless there are other suggestions, I'll likely submit this upstream. I didn't implement the -P flag since that's the default behavior. Thanks, Shawn On Wed, Feb 9, 2011 at 2:10 PM, Shawn Webb <lattera@gmail.com> wrote: > Included in the attached patch is the refactor using fts(3) and with the -L > and -H options. I'm still looking for feedback and suggestions on how to > improve the patch. I'll also port these changes over to my getfacl patch. > > If anyone's interested in following up-to-date development of the patch, > the link to it on github is > https://github.com/lattera/patches/blob/master/freebsd/setfacl_recursive.patch > > I'd like to take the time to address why I created the > remove_invalid_inherit function since I got a private email asking why it > existed. Other than symbolic links, non-directory entries cannot have > inheritance set. That function prevents attempting to set inheritance flags > on non-directory entries when doing a recursive call. That way, you can run > `setfacl -R -m user:<user>:read_data:file_inherit/dir_inherit:allow > <directory>` and not run into errors. > > Thanks, > > Shawn > > > On Tue, Feb 8, 2011 at 7:51 PM, Shawn Webb <lattera@gmail.com> wrote: > >> On Tue, Feb 8, 2011 at 7:35 PM, Tim Kientzle <tim@kientzle.com> wrote: >> >>> On Feb 8, 2011, at 9:58 AM, Shawn Webb wrote: >>> > I've just finished a patch to add recursive functionality to setfacl. >>> Before >>> > I officially submit it, I'd like a few suggestions on how to improve >>> the >>> > patch. >>> > >>> > The part I'm worried about involves the #define directive at top. I'm >>> not >>> > sure what ramifications using that define might have. I needed it for >>> my >>> > remove_invalid_inherit() function to work. >>> >>> You should certainly not need >>> #define _ACL_PRIVATE >>> for any user-space utilities. What exactly is the >>> problem without that? >>> >>> Your approach to directory walking here >>> is a little simplistic. In particular, you're storing >>> every filename for the entire tree in memory, >>> which is a problem for large filesystems. >>> >>> It would be much better to refactor the code so that >>> the actual ACL update was in a function and then >>> recurse_directory should call that function for >>> each filename as it visited it. That will reduce >>> the memory requirements significantly. >>> >>> You should also take a look at fts(3). In particular, >>> you'll want to implement the BSD-standard >>> -L/-P/-H options, and fts(3) makes that much easier. >>> (-L always follows symlinks, -P never follows symlinks, >>> -H follows symlinks on the command line). >>> >>> Tim >>> >>> >> Great suggestions. I'll definitely look at implementing that >> functionality. >> >> As a side note, it looks like my setfacl patch segfaults on >> freebsd-current r218075 with the zpool v28 patchset applied. I wrote it on >> freebsd 8.2-RC3 with zpool v15. >> >> Thanks, >> >> Shawn >> > > --0016e659f5f45082b9049bf02fe6 Content-Type: application/octet-stream; name="setfacl_recursive.patch" Content-Disposition: attachment; filename="setfacl_recursive.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gjzw57un1 LS0tIC91c3Ivc3JjL2Jpbi9zZXRmYWNsL3NldGZhY2wuYwkyMDExLTAyLTAzIDEyOjExOjAyLjMw MzQ5NjMxOCAtMDcwMAorKysgYmluL3NldGZhY2wvc2V0ZmFjbC5jCTIwMTEtMDItMTAgMDk6MTg6 NTIuMzgxMjgzODU4IC0wNzAwCkBAIC0zMiw2ICszMiw4IEBACiAjaW5jbHVkZSA8c3lzL3N0YXQu aD4KICNpbmNsdWRlIDxzeXMvYWNsLmg+CiAjaW5jbHVkZSA8c3lzL3F1ZXVlLmg+CisjaW5jbHVk ZSA8ZnRzLmg+CisjaW5jbHVkZSA8ZGlyZW50Lmg+CiAKICNpbmNsdWRlIDxlcnIuaD4KICNpbmNs dWRlIDxlcnJuby5oPgpAQCAtNDQsNiArNDYsOCBAQAogCiBzdGF0aWMgdm9pZAlhZGRfZmlsZW5h bWUoY29uc3QgY2hhciAqZmlsZW5hbWUpOwogc3RhdGljIHZvaWQJdXNhZ2Uodm9pZCk7CitzdGF0 aWMgdm9pZAlyZWN1cnNlX2RpcmVjdG9yeShjaGFyICpjb25zdCAqcGF0aHMsIGludCByX2ZsYWcs IGludCBsX2ZsYWcsIGludCBiaWdfaF9mbGFnKTsKK3N0YXRpYyBhY2xfdAlyZW1vdmVfaW52YWxp ZF9pbmhlcml0KGNvbnN0IGNoYXIgKnBhdGgsIGFjbF90IGFjbCwgaW50IGxfZmxhZyk7CiAKIHN0 YXRpYyB2b2lkCiBhZGRfZmlsZW5hbWUoY29uc3QgY2hhciAqZmlsZW5hbWUpCkBAIC02MiwzNCAr NjYsMTEyIEBACiBzdGF0aWMgdm9pZAogdXNhZ2Uodm9pZCkKIHsKLQotCWZwcmludGYoc3RkZXJy LCAidXNhZ2U6IHNldGZhY2wgWy1iZGhrbl0gWy1hIHBvc2l0aW9uIGVudHJpZXNdICIKKwlmcHJp bnRmKHN0ZGVyciwgInVzYWdlOiBzZXRmYWNsIFstYmRoa0xuUl0gWy1hIHBvc2l0aW9uIGVudHJp ZXNdICIKIAkgICAgIlstbSBlbnRyaWVzXSBbLU0gZmlsZV0gWy14IGVudHJpZXNdIFstWCBmaWxl XSBbZmlsZSAuLi5dXG4iKTsKIAlleGl0KDEpOwogfQogCitzdGF0aWMgdm9pZAorcmVjdXJzZV9k aXJlY3RvcnkoY2hhciAqY29uc3QgKnBhdGhzLCBpbnQgcl9mbGFnLCBpbnQgbF9mbGFnLCBpbnQg YmlnX2hfZmxhZykKK3sKKwlGVFMgKmZ0c3A7CisJRlRTRU5UICpwLCAqY2hwOworCWludCBmdHNf b3B0aW9ucyA9IEZUU19OT0NIRElSOworCXVuc2lnbmVkIGludCBpOworCQorCWZ0c19vcHRpb25z IHw9IChsX2ZsYWcgPT0gMSkgPyBGVFNfTE9HSUNBTCA6IEZUU19QSFlTSUNBTDsKKwlpZiAoYmln X2hfZmxhZykKKwkJZnRzX29wdGlvbnMgfD0gRlRTX0NPTUZPTExPVzsKKwkKKwlpZiAocl9mbGFn KQorCXsKKwkJZnRzcCA9IGZ0c19vcGVuKHBhdGhzLCBmdHNfb3B0aW9ucywgTlVMTCk7CisJCWlm IChmdHNwID09IE5VTEwpCisJCQlyZXR1cm47CisJCQorCQljaHAgPSBmdHNfY2hpbGRyZW4oZnRz cCwgMCk7CisJCWlmIChjaHAgPT0gTlVMTCkKKwkJCXJldHVybjsKKwkJCisJCXdoaWxlICgocCA9 IGZ0c19yZWFkKGZ0c3ApKSAhPSBOVUxMKSB7CisJCQlpZiAobF9mbGFnID09IDAgJiYgcC0+ZnRz X2luZm8gJiBGVFNfRCkKKwkJCQljb250aW51ZTsKKwkJCWVsc2UgaWYgKGxfZmxhZyA9PSAxICYm IHAtPmZ0c19pbmZvICYgRlRTX0RQKQorCQkJCWNvbnRpbnVlOworCQkJCisJCQlhZGRfZmlsZW5h bWUoc3RyZHVwKHAtPmZ0c19wYXRoKSk7CisJCX0KKwkJCisJCWZ0c19jbG9zZShmdHNwKTsKKwl9 IGVsc2UKKwkJZm9yIChpID0gMDsgcGF0aHNbaV0gIT0gTlVMTDsgaSsrKQorCQkJYWRkX2ZpbGVu YW1lKHBhdGhzW2ldKTsKK30KKworc3RhdGljIGFjbF90CityZW1vdmVfaW52YWxpZF9pbmhlcml0 KGNvbnN0IGNoYXIgKnBhdGgsIGFjbF90IGFjbCwgaW50IGxfZmxhZykKK3sKKwlhY2xfdCBhY2xf bmV3OworCWludCBhY2xfYnJhbmQ7CisJYWNsX2VudHJ5X3QgZW50cnk7CisJaW50IGVudHJ5X2lk OworCWFjbF9mbGFnc2V0X3QgZmxhZ3NldDsKKwlzdHJ1Y3Qgc3RhdCBzYjsKKwkKKwlhY2xfZ2V0 X2JyYW5kX25wKGFjbCwgJmFjbF9icmFuZCk7CisJaWYgKGFjbF9icmFuZCAhPSBBQ0xfQlJBTkRf TkZTNCkKKwkJcmV0dXJuIGFjbDsKKwkKKwlpZiAobF9mbGFnID09IDEpIHsKKwkJaWYgKHN0YXQo cGF0aCwgJnNiKSA9PSAtMSkKKwkJCXJldHVybiBhY2w7CisJfSBlbHNlCisJCWlmIChsc3RhdChw YXRoLCAmc2IpID09IC0xKQorCQkJcmV0dXJuIGFjbDsKKwkKKwlpZiAoU19JU0RJUihzYi5zdF9t b2RlKSAhPSAwKQorCQlyZXR1cm4gYWNsOworCQorCWFjbF9uZXcgPSBhY2xfZHVwKGFjbCk7CisJ CisJZW50cnlfaWQgPSBBQ0xfRklSU1RfRU5UUlk7CisJd2hpbGUgKGFjbF9nZXRfZW50cnkoYWNs X25ldywgZW50cnlfaWQsICZlbnRyeSkgPT0gMSkgeworCQllbnRyeV9pZCA9IEFDTF9ORVhUX0VO VFJZOworCQlhY2xfZ2V0X2ZsYWdzZXRfbnAoZW50cnksICZmbGFnc2V0KTsKKwkJaWYgKGFjbF9n ZXRfZmxhZ19ucChmbGFnc2V0LCBBQ0xfRU5UUllfSU5IRVJJVF9PTkxZKSkgeworCQkJYWNsX2Rl bGV0ZV9lbnRyeShhY2xfbmV3LCBlbnRyeSk7CisJCQljb250aW51ZTsKKwkJfQorCQlhY2xfZGVs ZXRlX2ZsYWdfbnAoZmxhZ3NldCwgQUNMX0VOVFJZX0ZJTEVfSU5IRVJJVCB8IEFDTF9FTlRSWV9E SVJFQ1RPUllfSU5IRVJJVCB8IEFDTF9FTlRSWV9OT19QUk9QQUdBVEVfSU5IRVJJVCk7CisJfQor CQorCXJldHVybiBhY2xfbmV3OworfQorCiBpbnQKIG1haW4oaW50IGFyZ2MsIGNoYXIgKmFyZ3Zb XSkKIHsKLQlhY2xfdCBhY2w7CisJYWNsX3QgYWNsLCBhY2xfYmFja3VwOwogCWFjbF90eXBlX3Qg YWNsX3R5cGU7CiAJY2hhciBmaWxlbmFtZVtQQVRIX01BWF07Ci0JaW50IGxvY2FsX2Vycm9yLCBj YXJyaWVkX2Vycm9yLCBjaCwgaSwgZW50cnlfbnVtYmVyLCByZXQ7Ci0JaW50IGhfZmxhZzsKKwlp bnQgbG9jYWxfZXJyb3IsIGNhcnJpZWRfZXJyb3IsIGNoLCBlbnRyeV9udW1iZXIsIHJldDsKKwlp bnQgaF9mbGFnLCByX2ZsYWcsIGxfZmxhZywgYmlnX2hfZmxhZzsKIAlzdHJ1Y3Qgc2ZfZmlsZSAq ZmlsZTsKIAlzdHJ1Y3Qgc2ZfZW50cnkgKmVudHJ5OwotCWNvbnN0IGNoYXIgKmZuX2R1cDsKKwlj aGFyICpmbl9kdXA7CiAJY2hhciAqZW5kOworCWNoYXIgKipmaWxlcz1OVUxMOworCXVuc2lnbmVk IGludCBudW1maWxlcz0wOwogCXN0cnVjdCBzdGF0IHNiOwogCiAJYWNsX3R5cGUgPSBBQ0xfVFlQ RV9BQ0NFU1M7CiAJY2FycmllZF9lcnJvciA9IGxvY2FsX2Vycm9yID0gMDsKLQloX2ZsYWcgPSBo YXZlX21hc2sgPSBoYXZlX3N0ZGluID0gbl9mbGFnID0gbmVlZF9tYXNrID0gMDsKKwloX2ZsYWcg PSBoYXZlX21hc2sgPSBoYXZlX3N0ZGluID0gbl9mbGFnID0gbmVlZF9tYXNrID0gcl9mbGFnID0g bF9mbGFnID0gYmlnX2hfZmxhZyA9IDA7CiAKIAlUQUlMUV9JTklUKCZlbnRyeWxpc3QpOwogCVRB SUxRX0lOSVQoJmZpbGVsaXN0KTsKIAotCXdoaWxlICgoY2ggPSBnZXRvcHQoYXJnYywgYXJndiwg Ik06WDphOmJkaGttOm54OiIpKSAhPSAtMSkKKwl3aGlsZSAoKGNoID0gZ2V0b3B0KGFyZ2MsIGFy Z3YsICJITFJNOlg6YTpiZGhrbTpueDoiKSkgIT0gLTEpCiAJCXN3aXRjaChjaCkgewogCQljYXNl ICdNJzoKIAkJCWVudHJ5ID0gem1hbGxvYyhzaXplb2Yoc3RydWN0IHNmX2VudHJ5KSk7CkBAIC0x NjcsNiArMjQ5LDE1IEBACiAJCQl9CiAJCQlUQUlMUV9JTlNFUlRfVEFJTCgmZW50cnlsaXN0LCBl bnRyeSwgbmV4dCk7CiAJCQlicmVhazsKKwkJY2FzZSAnUic6CisJCQlyX2ZsYWcgPSAxOworCQkJ YnJlYWs7CisJCWNhc2UgJ0wnOgorCQkJbF9mbGFnID0gMTsKKwkJCWJyZWFrOworCQljYXNlICdI JzoKKwkJCWJpZ19oX2ZsYWcgPSAxOworCQkJYnJlYWs7CiAJCWRlZmF1bHQ6CiAJCQl1c2FnZSgp OwogCQkJYnJlYWs7CkBAIC0xODksMTEgKzI4MCwxOCBAQAogCQkJZm5fZHVwID0gc3RyZHVwKGZp bGVuYW1lKTsKIAkJCWlmIChmbl9kdXAgPT0gTlVMTCkKIAkJCQllcnIoMSwgInN0cmR1cCgpIGZh aWxlZCIpOwotCQkJYWRkX2ZpbGVuYW1lKGZuX2R1cCk7CisJCQlmaWxlcyA9IHJlYWxsb2MoZmls ZXMsICsrbnVtZmlsZXMgKiBzaXplb2YoY2hhciAqKikpOworCQkJaWYgKGZpbGVzID09IE5VTEwp CisJCQkJZXJyKDEsICJyZWFsbG9jKCkgZmFpbGVkIik7CisJCQlmaWxlc1tudW1maWxlcy0xXSA9 IChjaGFyICopZm5fZHVwOwogCQl9CisJCQorCQlmaWxlcyA9IHJlYWxsb2MoZmlsZXMsICsrbnVt ZmlsZXMgKiBzaXplb2YoY2hhciAqKikpOworCQlmaWxlc1tudW1maWxlcy0xXSA9IE5VTEw7CiAJ fSBlbHNlCi0JCWZvciAoaSA9IDA7IGkgPCBhcmdjOyBpKyspCi0JCQlhZGRfZmlsZW5hbWUoYXJn dltpXSk7CisJCWZpbGVzID0gYXJndjsKKwkKKwlyZWN1cnNlX2RpcmVjdG9yeShmaWxlcywgcl9m bGFnLCBsX2ZsYWcsIGJpZ19oX2ZsYWcpOwogCiAJLyogY3ljbGUgdGhyb3VnaCBlYWNoIGZpbGUg Ki8KIAlUQUlMUV9GT1JFQUNIKGZpbGUsICZmaWxlbGlzdCwgbmV4dCkgewpAQCAtMjUwLDEyICsz NDgsMjQgQEAKIAogCQkJc3dpdGNoKGVudHJ5LT5vcCkgewogCQkJY2FzZSBPUF9BRERfQUNMOgor CQkJCWFjbF9iYWNrdXAgPSBlbnRyeS0+YWNsOworCQkJCWVudHJ5LT5hY2wgPSByZW1vdmVfaW52 YWxpZF9pbmhlcml0KGZpbGUtPmZpbGVuYW1lLCBlbnRyeS0+YWNsLCBsX2ZsYWcpOwogCQkJCWxv Y2FsX2Vycm9yICs9IGFkZF9hY2woZW50cnktPmFjbCwKIAkJCQkgICAgZW50cnktPmVudHJ5X251 bWJlciwgJmFjbCwgZmlsZS0+ZmlsZW5hbWUpOworCQkJCWlmIChlbnRyeS0+YWNsICE9IGFjbF9i YWNrdXApIHsKKwkJCQkJYWNsX2ZyZWUoZW50cnktPmFjbCk7CisJCQkJCWVudHJ5LT5hY2wgPSBh Y2xfYmFja3VwOworCQkJCX0KIAkJCQlicmVhazsKIAkJCWNhc2UgT1BfTUVSR0VfQUNMOgorCQkJ CWFjbF9iYWNrdXAgPSBlbnRyeS0+YWNsOworCQkJCWVudHJ5LT5hY2wgPSByZW1vdmVfaW52YWxp ZF9pbmhlcml0KGZpbGUtPmZpbGVuYW1lLCBlbnRyeS0+YWNsLCBsX2ZsYWcpOwogCQkJCWxvY2Fs X2Vycm9yICs9IG1lcmdlX2FjbChlbnRyeS0+YWNsLCAmYWNsLAogCQkJCSAgICBmaWxlLT5maWxl bmFtZSk7CisJCQkJaWYgKGVudHJ5LT5hY2wgIT0gYWNsX2JhY2t1cCkgeworCQkJCQlhY2xfZnJl ZShlbnRyeS0+YWNsKTsKKwkJCQkJZW50cnktPmFjbCA9IGFjbF9iYWNrdXA7CisJCQkJfQogCQkJ CW5lZWRfbWFzayA9IDE7CiAJCQkJYnJlYWs7CiAJCQljYXNlIE9QX1JFTU9WRV9FWFQ6Cg== --0016e659f5f45082b9049bf02fe6--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTik4B1O=xx1MmjbdMVO6pwR=MaBjVaWU_bi4Ry8c>