On Mon, May 17, 2021 at 10:53:55AM -0700, Sargun Dhillon wrote: > On Tue, May 11, 2021 at 2:50 PM Tycho Andersen <tycho@tycho.pizza> wrote: > > > The struct seccomp_notif_resp, used when doing SECCOMP_IOCTL_NOTIF_SEND > > > ioctl() to send a response to the target, has three more fields that we > > > don't allow to set when doing the addfd ioctl() to also return. The > > > reasons to disallow each field are: > > > * val: This will be set to the new allocated fd. No point taking it > > > from userspace in this case. > > > * error: If this is non-zero, the value is ignored. Therefore, > > > it is pointless in this case as we want to return the value. > > > * flags: The only flag is to let userspace continue to execute the > > > syscall. This seems pointless, as we want the syscall to return the > > > allocated fd. > > > > > > This is why those fields are not possible to set when using this new > > > flag. > > > > I don't quite understand this; you don't need a NOTIF_SEND at all > > with the way this currently works, right? > > > I reworded: > > This effectively combines SECCOMP_IOCTL_NOTIF_ADDFD and > SECCOMP_IOCTL_NOTIF_SEND into an atomic opteration. The notification's > return value, nor error can be set by the user. Upon successful invocation > of the SECCOMP_IOCTL_NOTIF_ADDFD ioctl with the SECCOMP_ADDFD_FLAG_SEND > flag, the notifying process's errno will be 0, and the return value will > be the file descriptor number that was installed. > > How does that sound? Works for me, thanks! > > > @@ -1113,7 +1136,7 @@ static int seccomp_do_user_notification(int this_syscall, > > > struct seccomp_kaddfd, list); > > > /* Check if we were woken up by a addfd message */ > > > if (addfd) > > > - seccomp_handle_addfd(addfd); > > > + seccomp_handle_addfd(addfd, &n); > > > > > > } while (n.state != SECCOMP_NOTIFY_REPLIED); > > > > > > > This while() bit is introduced in the previous patch, can we fold this > > deletion into that somehow? > I'm not sure what you're getting at. This just an argument change which > also passes the notification to the addfd function. The patch is split out > to allow it to be backported to stable. Yeah, I was mis-reading, you can ignore this. Sorry for the noise. If you send another version, you can call the series: Acked-by: Tycho Andersen <tycho@tycho.pizza> Tycho
