On Thu, Sep 24, 2026 at 02:57:47PM -0400, [email protected] wrote: > From: Hengyu Liang <[email protected]> > > Before commit 18e2bf0edf4d ("eventpoll: Add epoll ioctl for > epoll_params"), epoll files had no ioctl handler, so ioctl() on an epoll > file descriptor failed with ENOTTY. That commit introduced the > EPIOCSPARAMS and EPIOCGPARAMS commands, but ep_eventpoll_ioctl() returns > -EINVAL for any other command, so since v6.9 every other ioctl() on an > epoll file descriptor fails with EINVAL instead of ENOTTY. > > Documentation/driver-api/ioctl.rst says that an ioctl handler must > return -ENOTTY or -ENOIOCTLCMD for an unknown command, and that > returning -EINVAL there is wrong. Returning -ENOIOCTLCMD was also the > intent of the original series, whose changelog since v3 [1] says "when > an unknown ioctl is received, -ENOIOCTLCMD is returned instead of > -EINVAL as the ioctl documentation requires", and ep_eventpoll_bp_ioctl() > does return -ENOIOCTLCMD for unknown commands. However, > ep_eventpoll_ioctl() only passes EPIOCSPARAMS and EPIOCGPARAMS to it and > handles all other commands in its own default case, which returns > -EINVAL, so that path is never reached. > > This is visible to userspace. For example, isatty(), ttyname() and > tcgetattr() on an epoll file descriptor set errno to EINVAL, while they > set ENOTTY for any other file descriptor that does not refer to a > terminal, as they also did for epoll file descriptors before v6.9. > > Return -ENOIOCTLCMD from the default case, which the VFS turns into > -ENOTTY, and update the epoll_busy_poll selftest, which expected EINVAL > for an unknown command. > > [1] https://lore.kernel.org/r/[email protected] > > Fixes: 18e2bf0edf4d ("eventpoll: Add epoll ioctl for epoll_params") > Signed-off-by: Hengyu Liang <[email protected]> > --- > fs/eventpoll.c | 2 +- > tools/testing/selftests/net/epoll_busy_poll.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > index e0c4bf88a838..adf30b720b13 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -1264,7 +1264,7 @@ static long ep_eventpoll_ioctl(struct file *file, > unsigned int cmd, > ret = ep_eventpoll_bp_ioctl(file, cmd, arg); > break; > default: > - ret = -EINVAL; > + ret = -ENOIOCTLCMD; > break; > }
I think based on the documentation this is probably right, but I am now wondering why both ep_eventpoll_ioctl and ep_eventpoll_bp_ioctl need to exist. Maybe when I first implemented this I thought it made sense to factor out the busy poll ioctls into their own function, but in retrospect maybe it's cleaner to just collapse the ioctl function into a single one instead of having two layers? In other words, maybe: - delete ep_eventpoll_ioctl - add the is_file_epoll check to ep_eventpoll_bp_ioctl - rename ep_eventpoll_bp_ioctl to ep_eventpoll_ioctl - fix the test (as you did in this version of the patch) Would result in a cleaner fewer helpers / cleaner code ?
