| Message ID | 9293c7ee-6fb7-7142-66fe-051548ffb65c@ya.ru (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | af_unix: Add ioctl(SIOCUNIXGRABFDS) to grab files of receive queue skbs | expand |
Hi Kirill,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on net-next/master linus/master v5.19 next-20220812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kirill-Tkhai/af_unix-Add-ioctl-SIOCUNIXGRABFDS-to-grab-files-of-receive-queue-skbs/20220815-045608
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 777885673122b78b2abd2f1e428730961a786ff2
config: s390-randconfig-r044-20220815 (https://download.01.org/0day-ci/archive/20220815/202208150743.t05nZxqC-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 3329cec2f79185bafd678f310fafadba2a8c76d2)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/0b4bc309fb3cdc6e470ee5c28e33f2909bfb8266
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kirill-Tkhai/af_unix-Add-ioctl-SIOCUNIXGRABFDS-to-grab-files-of-receive-queue-skbs/20220815-045608
git checkout 0b4bc309fb3cdc6e470ee5c28e33f2909bfb8266
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
ERROR: modpost: "devm_ioremap" [drivers/net/ethernet/altera/altera_tse.ko] undefined!
>> ERROR: modpost: "__receive_fd" [net/unix/unix.ko] undefined!
Hi Kirill,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on net-next/master linus/master horms-ipvs/master v6.0-rc2 next-20220819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kirill-Tkhai/af_unix-Add-ioctl-SIOCUNIXGRABFDS-to-grab-files-of-receive-queue-skbs/20220815-045608
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 777885673122b78b2abd2f1e428730961a786ff2
config: i386-randconfig-s003 (https://download.01.org/0day-ci/archive/20220822/202208221324.VmNeMI4S-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/0b4bc309fb3cdc6e470ee5c28e33f2909bfb8266
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kirill-Tkhai/af_unix-Add-ioctl-SIOCUNIXGRABFDS-to-grab-files-of-receive-queue-skbs/20220815-045608
git checkout 0b4bc309fb3cdc6e470ee5c28e33f2909bfb8266
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash arch/x86/entry/ fs/cifs/ net/unix/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> net/unix/af_unix.c:3130:69: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected int [noderef] __user *ufd @@ got int * @@
net/unix/af_unix.c:3130:69: sparse: expected int [noderef] __user *ufd
net/unix/af_unix.c:3130:69: sparse: got int *
net/unix/af_unix.c:159:13: sparse: sparse: context imbalance in 'unix_table_double_lock' - wrong count at exit
net/unix/af_unix.c:178:28: sparse: sparse: context imbalance in 'unix_table_double_unlock' - unexpected unlock
net/unix/af_unix.c:1290:13: sparse: sparse: context imbalance in 'unix_state_double_lock' - wrong count at exit
net/unix/af_unix.c:1308:17: sparse: sparse: context imbalance in 'unix_state_double_unlock' - unexpected unlock
net/unix/af_unix.c:1609:18: sparse: sparse: context imbalance in 'unix_stream_connect' - different lock contexts for basic block
net/unix/af_unix.c:1972:25: sparse: sparse: context imbalance in 'unix_dgram_sendmsg' - unexpected unlock
net/unix/af_unix.c:3325:20: sparse: sparse: context imbalance in 'unix_get_first' - wrong count at exit
net/unix/af_unix.c:3356:34: sparse: sparse: context imbalance in 'unix_get_next' - unexpected unlock
net/unix/af_unix.c:3386:42: sparse: sparse: context imbalance in 'unix_seq_stop' - unexpected unlock
net/unix/af_unix.c:3489:34: sparse: sparse: context imbalance in 'bpf_iter_unix_hold_batch' - unexpected unlock
vim +3130 net/unix/af_unix.c
3081
3082 static int unix_ioc_grab_fds(struct sock *sk, struct unix_ioc_grab_fds __user *uarg)
3083 {
3084 int i, todo, skip, count, all, err, done = 0;
3085 struct unix_sock *u = unix_sk(sk);
3086 struct unix_ioc_grab_fds arg;
3087 struct sk_buff *skb = NULL;
3088 struct scm_fp_list *fp;
3089
3090 if (copy_from_user(&arg, uarg, sizeof(arg)))
3091 return -EFAULT;
3092
3093 skip = arg.in.nr_skip;
3094 todo = arg.in.nr_grab;
3095
3096 if (skip < 0 || todo <= 0)
3097 return -EINVAL;
3098 if (mutex_lock_interruptible(&u->iolock))
3099 return -EINTR;
3100
3101 all = atomic_read(&u->scm_stat.nr_fds);
3102 err = -EFAULT;
3103 /* Set uarg->out.nr_all before the first file is received. */
3104 if (put_user(all, &uarg->out.nr_all))
3105 goto unlock;
3106 err = 0;
3107 if (all <= skip)
3108 goto unlock;
3109 if (all - skip < todo)
3110 todo = all - skip;
3111 while (todo) {
3112 spin_lock(&sk->sk_receive_queue.lock);
3113 if (!skb)
3114 skb = skb_peek(&sk->sk_receive_queue);
3115 else
3116 skb = skb_peek_next(skb, &sk->sk_receive_queue);
3117 spin_unlock(&sk->sk_receive_queue.lock);
3118
3119 if (!skb)
3120 goto unlock;
3121
3122 fp = UNIXCB(skb).fp;
3123 count = fp->count;
3124 if (skip >= count) {
3125 skip -= count;
3126 continue;
3127 }
3128
3129 for (i = skip; i < count && todo; i++) {
> 3130 err = receive_fd_user(fp->fp[i], &arg.in.fds[done], 0);
3131 if (err < 0)
3132 goto unlock;
3133 done++;
3134 todo--;
3135 }
3136 skip = 0;
3137 }
3138 unlock:
3139 mutex_unlock(&u->iolock);
3140
3141 /* Return number of fds (non-error) if there is a received file. */
3142 if (done)
3143 return done;
3144 if (err < 0)
3145 return err;
3146 return 0;
3147 }
3148
diff --git a/include/uapi/linux/un.h b/include/uapi/linux/un.h index 0ad59dc8b686..995b358263dd 100644 --- a/include/uapi/linux/un.h +++ b/include/uapi/linux/un.h @@ -11,6 +11,18 @@ struct sockaddr_un { char sun_path[UNIX_PATH_MAX]; /* pathname */ }; +struct unix_ioc_grab_fds { + struct { + int nr_grab; + int nr_skip; + int *fds; + } in; + struct { + int nr_all; + } out; +}; + #define SIOCUNIXFILE (SIOCPROTOPRIVATE + 0) /* open a socket file with O_PATH */ +#define SIOCUNIXGRABFDS (SIOCPROTOPRIVATE + 1) /* grab files from recv queue */ #endif /* _LINUX_UN_H */ diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index bf338b782fc4..3c7e8049eba1 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -3079,6 +3079,73 @@ static int unix_open_file(struct sock *sk) return fd; } +static int unix_ioc_grab_fds(struct sock *sk, struct unix_ioc_grab_fds __user *uarg) +{ + int i, todo, skip, count, all, err, done = 0; + struct unix_sock *u = unix_sk(sk); + struct unix_ioc_grab_fds arg; + struct sk_buff *skb = NULL; + struct scm_fp_list *fp; + + if (copy_from_user(&arg, uarg, sizeof(arg))) + return -EFAULT; + + skip = arg.in.nr_skip; + todo = arg.in.nr_grab; + + if (skip < 0 || todo <= 0) + return -EINVAL; + if (mutex_lock_interruptible(&u->iolock)) + return -EINTR; + + all = atomic_read(&u->scm_stat.nr_fds); + err = -EFAULT; + /* Set uarg->out.nr_all before the first file is received. */ + if (put_user(all, &uarg->out.nr_all)) + goto unlock; + err = 0; + if (all <= skip) + goto unlock; + if (all - skip < todo) + todo = all - skip; + while (todo) { + spin_lock(&sk->sk_receive_queue.lock); + if (!skb) + skb = skb_peek(&sk->sk_receive_queue); + else + skb = skb_peek_next(skb, &sk->sk_receive_queue); + spin_unlock(&sk->sk_receive_queue.lock); + + if (!skb) + goto unlock; + + fp = UNIXCB(skb).fp; + count = fp->count; + if (skip >= count) { + skip -= count; + continue; + } + + for (i = skip; i < count && todo; i++) { + err = receive_fd_user(fp->fp[i], &arg.in.fds[done], 0); + if (err < 0) + goto unlock; + done++; + todo--; + } + skip = 0; + } +unlock: + mutex_unlock(&u->iolock); + + /* Return number of fds (non-error) if there is a received file. */ + if (done) + return done; + if (err < 0) + return err; + return 0; +} + static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) { struct sock *sk = sock->sk; @@ -3113,6 +3180,9 @@ static int unix_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) } break; #endif + case SIOCUNIXGRABFDS: + err = unix_ioc_grab_fds(sk, (struct unix_ioc_grab_fds __user *)arg); + break; default: err = -ENOIOCTLCMD; break;
When a fd owning a counter of some critical resource, say, of a mount, it's impossible to umount that mount and disconnect related block device. That fd may be contained in some unix socket receive queue skb. Despite we have an interface for detecting such the sockets queues (/proc/[PID]/fdinfo/[fd] shows non-zero scm_fds count if so) and it's possible to kill that process to release the counter, the problem is that there may be several processes, and it's not a good thing to kill each of them. This patch adds a simple interface to grab files from receive queue, so the caller may analyze them, and even do that recursively, if grabbed file is unix socket itself. So, the described above problem may be solved by this ioctl() in pair with pidfd_getfd(). Note, that the existing recvmsg(,,MSG_PEEK) is not suitable for that purpose, since it modifies peek offset inside socket, and this results in a problem in case of examined process uses peek offset itself. Additional ptrace freezing of that task plus ioctl(SO_PEEK_OFF) won't help too, since that socket may relate to several tasks, and there is no reliable and non-racy way to detect that. Also, if the caller of such trick will die, the examined task will remain frozen forever. The new suggested ioctl(SIOCUNIXGRABFDS) does not have such problems. The realization of ioctl(SIOCUNIXGRABFDS) is pretty simple. The only interesting thing is protocol with userspace. Firstly, we let userspace to know the number of all files in receive queue skbs. Then we receive fds one by one starting from requested offset. We return number of received fds if there is a successfully received fd, and this number may be less in case of error or desired fds number lack. Userspace may detect that situations by comparison of returned value and out.nr_all minus in.nr_skip. Looking over different variant this one looks the best for me (I considered returning error in case of error and there is a received fd. Also I considered returning number of received files as one more member in struct unix_ioc_grab_fds). Signed-off-by: Kirill Tkhai <tkhai@ya.ru> --- include/uapi/linux/un.h | 12 ++++++++ net/unix/af_unix.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+)