Thanks for another detailed explanation. > And generally all patches that are committed and pushed are also available on > kernel.org grouped by release. I've been trying my best to ensure they is as > accurate as possible so it is clear what has gone into each release. I'm assuming you're talking about https://www.kernel.org/pub/linux/daemons/autofs/ for userspace stuff. > Also, at times I have tried to put together a TODO list of longer term things > that I'd like to do for myself but haven't been very successful, ;) It would be cool to know the long term goal, if you can (or want to) make it public. 2016-07-21 11:02 GMT+09:00 Ian Kent <raven@xxxxxxxxxx>: > On Thu, 2016-07-21 at 02:53 +0900, Tomohiro Kusumi wrote: >> > So I think the problem with the miscellaneous device ioctl number check >> > you've >> > pointed out does need to be fixed and if you could update the patch to do >> > that >> > it would be much appreciated. >> >> OK, I'll take a look at it again based on your explanation, >> and see if I can fix it, plus make things a bit more clear if possible. >> >> > Don't forget that the miscellaneous device ioctls are a newer set of ioctls >> > with >> > added functionality to the older ioctls, hence the duplication you see with >> > the >> > functions provided with each set, and the check for that specific range is >> > relevant to the later ones only. >> > >> > Also, only one or the other set of ioctls can be used and the miscellaneous >> > device ioctls can only be used with version 5 and above of user space >> > autofs. >> >> Yes, I was wondering why they're implemented to both fs root and /dev/autofs >> when I first looked at ioctl and userspace code. >> >> > >> > I'd like to try (again) to re-implement this with netlink and add readdir >> > and >> > asynchronous expire functionality but I don't know when I'll get time to do >> > it s >> > ince it is a lot of work in kernel and user space, but I digress. >> >> Is there a publicly available list of TODO for autofs (kernel and userspace) ? >> >> According to some of your recent posts to this mailing list, >> you seem to have lots of patches that haven't been merged to upstream, >> and also things yet to be implemented. > > There isn't. > > The truth is there haven't been other developers that have stuck with it so I've > pretty much done everything in isolation. > > That's not too good because that means there isn't discussion about how things > should be done and there's very little review of changes too, since there's no > -one to review them. > > If there were others, such as yourself, I would start doing things quite > differently, but clearly that would take time to start happening. > > I detest writing and maintaining web presences so there is no wiki which is > another problem, and is the sort of place where a TODO list would reside. > > As far as patches that are either not finished, did not solve a problem, or are > in progress, yes I have plenty of those and they have accumulated over many > years. > > That's not uncommon when you work with something for a long time but they are not useful until you return to a problem they may be related too, so mostly not useful to others. > > For example, I tried to implement the control interface using the netlink > mechanism, a long time ago now, and failed but I keep the work that I did in > case I can use it at some time is the future, that's my nature. > > A netlink interface would be better than using ioctls, it would be more flexible > in the long run but, at the time, I was unwilling to fundamentally change the > way automount(8) works and there were other difficulties with asynchronous > expires so I failed to get it working and implemented the anonymous device ioctl > interface instead. > > So these patches are not useful at all and are just a consequence of things I > have tried to do in the past. > > But there is still the question of how to better deal with listing of > directories (using certain options) that can cause everything within the > directory to mount. > > Over the years I've thought about it many times and at times I think a kernel > readdir implementation would be better and (often after thinking more about it) > I come back to there being no advantage over the existing pre-creation of mount > point directories. > > If I was to, one day, believe that a kernel readdir implementation would make a > worthwhile difference then I think it would be better to implement that using > the netlink sub-system and the problems with that would need to be worked out to > do so. > > So it ends up being little more than something for thought only, or possibly > discussion if there were others to take part in it, and what I have for this > would be extremely hard for anyone else to use as it is a mess of partially done > work. > > For a long time now I accumulate patches and then commit and push those that > prove sound. And often it has been quite a while between pushes so there can end > up being quite a few, but that can change if there is reason to do so. > > And generally all patches that are committed and pushed are also available on > kernel.org grouped by release. I've been trying my best to ensure they is as > accurate as possible so it is clear what has gone into each release. > > Also, at times I have tried to put together a TODO list of longer term things > that I'd like to do for myself but haven't been very successful, ;) > >> >> >> 2016-07-20 9:47 GMT+09:00 Ian Kent <raven@xxxxxxxxxx>: >> > On Wed, 2016-07-20 at 04:29 +0900, Tomohiro Kusumi wrote: >> > > Thanks for your review, >> > > >> > > Yes, yours apparently works and that's exactly the right test based on >> > > currently existing ioctl cmds. >> > > I first thought about changing it like you did, but thought about >> > > meaning of AUTOFS_IOC_COUNT which is 32. >> > >> > I think the meaning of this is taken from Documentation/ioctl/ioctl >> > -number.txt. >> > >> > The define isn't actually used though, which is what I was saying. >> > >> > > >> > > I took this 32 as some sort of reserved range for autofs ioctl cmds, >> > > where the whole ioctl cmds are designed to be somewhere in 0x60-0x80 >> > > range, and /dev/autofs ioctl cmds are in 0x71-0x80 range. >> > >> > Yes, if ioctl numbers beyond 0x7e are to be used I think we need to update >> > the >> > above file. >> > >> > It's been a while since I did this but the highest miscellaneous device >> > ioctl >> > number is 0x7e so if new ioctls were needed the usage registration file >> > would >> > need to be updated and posted as a patch. >> > >> > > So in theory a potential ioctl cmd for /dev/autofs counting from the >> > > smallest cmd must be no larger than 32-0x11 (or 0x80-0x71). >> > > >> > > I've no idea what this reservation/limitation up to 0x80 is supposed >> > > to mean though, as ioctl has 8 bits for nr part. >> > > If you say this 32 is meaningless, I'll resubmit it with your proposed >> > > change. >> > >> > >> > The AUTOFS_IOC_COUNT should stay purely because it documents the "claimed" >> > autofs range (from 0x60-0x7f). Maybe a comment is needed to clear this up. >> > >> > But AUTOFS_DEV_IOCTL_IOC_COUNT is supposed to be used to check the passed in >> > miscellaneous device ioctl number is between 0x71-0x7f (or would if it was >> > correct). >> > >> > So I think the problem with the miscellaneous device ioctl number check >> > you've >> > pointed out does need to be fixed and if you could update the patch to do >> > that >> > it would be much appreciated. >> > >> > Don't forget that the miscellaneous device ioctls are a newer set of ioctls >> > with >> > added functionality to the older ioctls, hence the duplication you see with >> > the >> > functions provided with each set, and the check for that specific range is >> > relevant to the later ones only. >> > >> > Also, only one or the other set of ioctls can be used and the miscellaneous >> > device ioctls can only be used with version 5 and above of user space >> > autofs. >> > >> > I'd like to try (again) to re-implement this with netlink and add readdir >> > and >> > asynchronous expire functionality but I don't know when I'll get time to do >> > it s >> > ince it is a lot of work in kernel and user space, but I digress. >> > >> > > >> > > >> > > 0x60 0x71 0x80 >> > > -----+------------------+---------------+------------------> NR >> > > <----------------------------------> AUTOFS_IOC_COUNT (32) >> > > <---------------> AUTOFS_DEV_IOCTL_IOC_COUNT >> > >> > > 2016-07-19 10:34 GMT+09:00 Ian Kent <raven@xxxxxxxxxx>: >> > > > On Sun, 2016-07-10 at 17:07 +0900, Tomohiro Kusumi wrote: >> > > > > According to include/uapi/linux/auto_fs.h, >> > > > > include/uapi/linux/auto_fs4.h, >> > > > > and include/linux/auto_dev-ioctl.h, nr part of >> > > > > AUTOFS_IOC_XXX starts off from 0x60 and >> > > > > AUTOFS_DEV_IOCTL_XXX starts off from 0x71 which is 0x60+0x11. >> > > > > >> > > > > From the way above macros are defined, it seems >> > > > > AUTOFS_DEV_IOCTL_IOC_COUNT should be (0x20-0x11) instead of (0x20-11). >> > > > > (Otherwise it's hard to figure out where 11 comes from) >> > > > >> > > > You're correct, this is wrong but I think this change is also wrong. >> > > > >> > > > And so is the range check in dev-ioctl.c. >> > > > >> > > > There's no check for an ioctl number greater than the range claimed by >> > > > autofs >> > > > and the check in dev-ioctl.c seeks to check the number used is within >> > > > the >> > > > range >> > > > used by the dev ioctls. >> > > > >> > > > Essentially >> > > > >> > > > AUTOFS_DEV_IOCTL_VERSION_CMD - AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD = 0xd >> > > > >> > > > 14 ioctls ranging from 0x71 - 0x7e (inclusive). >> > > > >> > > > The range check in dev-ioctl.c is: >> > > > >> > > > if (_IOC_TYPE(command) != _IOC_TYPE(AUTOFS_DEV_IOCTL_IOC_FIRST) >> > > > || >> > > > cmd - cmd_first >= AUTOFS_DEV_IOCTL_IOC_COUNT) { >> > > > return -ENOTTY; >> > > > } >> > > > >> > > > so I think AUTOFS_DEV_IOCTL_IOC_COUNT should be: >> > > > >> > > > #define AUTOFS_DEV_IOCTL_IOC_COUNT \ >> > > > (AUTOFS_DEV_IOCTL_VERSION_CMD - >> > > > AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD) >> > > > >> > > > and the check should be: >> > > > >> > > > if (_IOC_TYPE(command) != _IOC_TYPE(AUTOFS_DEV_IOCTL_IOC_FIRST) >> > > > || >> > > > >> > > > cmd_first - cmd >= AUTOFS_DEV_IOCTL_IOC_COUNT) { >> > > > return >> > > > -ENOTTY; >> > > > } >> > > > >> > > > What do you think? >> > > > >> > > > > >> > > > > COUNT macros are being used to test distance of an ioctl command >> > > > > in question from the smallest one, so we don't want it to be >> > > > > larger than necessary. >> > > > > >> > > > > Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@xxxxxxxxx> >> > > > > --- >> > > > > fs/autofs4/autofs_i.h | 2 +- >> > > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > > >> > > > > diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h >> > > > > index 73e3d38..7ef4d08 100644 >> > > > > --- a/fs/autofs4/autofs_i.h >> > > > > +++ b/fs/autofs4/autofs_i.h >> > > > > @@ -20,7 +20,7 @@ >> > > > > #define AUTOFS_IOC_COUNT 32 >> > > > > >> > > > > #define AUTOFS_DEV_IOCTL_IOC_FIRST (AUTOFS_DEV_IOCTL_VERSION) >> > > > > -#define AUTOFS_DEV_IOCTL_IOC_COUNT (AUTOFS_IOC_COUNT - 11) >> > > > > +#define AUTOFS_DEV_IOCTL_IOC_COUNT (AUTOFS_IOC_COUNT - 0x11) >> > > > > >> > > > > #include <linux/kernel.h> >> > > > > #include <linux/slab.h> -- To unsubscribe from this list: send the line "unsubscribe autofs" in
