Re: [PATCH 2/2] net: socket: implement SO_DESCRIPTION
On Sun, Aug 16, 2020 at 11:15 AM Eric Dumazet wrote: > > 1) You also ignored what would happen at accept() time. > > Please test your patches with ASAN. Ouch, I will look into it - thanks for pointing that out & 3/ > > 2) Also, why is that description specific to sockets ? fcntl on struct file was deemed too intrusive - as far as fds are concerned, regular files and pipes have names and local processes to match against anyway we're left with mostly remote targets - one example was to preserve target host names after resolution (alas, I don't think there's a clean flow to hook that transparently)
Re: [PATCH 2/2] net: socket: implement SO_DESCRIPTION
On 8/15/20 11:23 AM, Pascal Bouchareine wrote: > This command attaches the zero terminated string in optval to the > socket for troubleshooting purposes. The free string is displayed in the > process fdinfo file for that fd (/proc//fdinfo/). > > One intended usage is to allow processes to self-document sockets > for netstat and friends to report > > We ignore optlen and constrain the string to a static max size > > 1) You also ignored what would happen at accept() time. Please test your patches with ASAN. 2) Also, why is that description specific to sockets ? 3) When a new socket option is added, it is customary to implement both setsockopt() and getsockopt() for things like CRIU.
Re: [PATCH 2/2] net: socket: implement SO_DESCRIPTION
Hi Pascal,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/perf/core]
[also build test WARNING on linux/master v5.8]
[cannot apply to security/next-testing linus/master next-20200814]
[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]
url:
https://github.com/0day-ci/linux/commits/Pascal-Bouchareine/proc-socket-attach-description-to-sockets/20200816-090222
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
d903b6d029d66e6478562d75ea18d89098f7b7e8
config: x86_64-randconfig-r006-20200816 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
ab9fc8bae805c785066779e76e7846aabad5609e)
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 x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> net/core/sock.c:831:5: warning: no previous prototype for function
>> 'sock_set_description' [-Wmissing-prototypes]
int sock_set_description(struct sock *sk, char __user *user_desc)
^
net/core/sock.c:831:1: note: declare 'static' if the function is not
intended to be used outside of this translation unit
int sock_set_description(struct sock *sk, char __user *user_desc)
^
static
1 warning generated.
vim +/sock_set_description +831 net/core/sock.c
830
> 831 int sock_set_description(struct sock *sk, char __user *user_desc)
832 {
833 char *old, *desc;
834
835 desc = strndup_user(user_desc, SK_MAX_DESC_SIZE,
GFP_KERNEL_ACCOUNT);
836 if (IS_ERR(desc))
837 return PTR_ERR(desc);
838
839 lock_sock(sk);
840 old = sk->sk_description;
841 sk->sk_description = desc;
842 release_sock(sk);
843
844 kfree(old);
845
846 return 0;
847 }
848
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 2/2] net: socket: implement SO_DESCRIPTION
Hi Pascal, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on tip/perf/core] [also build test WARNING on linux/master v5.8] [cannot apply to security/next-testing linus/master next-20200814] [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] url: https://github.com/0day-ci/linux/commits/Pascal-Bouchareine/proc-socket-attach-description-to-sockets/20200816-090222 base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git d903b6d029d66e6478562d75ea18d89098f7b7e8 config: i386-randconfig-s002-20200816 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-168-g9554805c-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> net/core/sock.c:831:5: sparse: sparse: symbol 'sock_set_description' was not >> declared. Should it be static? net/core/sock.c:2028:9: sparse: sparse: context imbalance in 'sk_clone_lock' - wrong count at exit net/core/sock.c:2032:6: sparse: sparse: context imbalance in 'sk_free_unlock_clone' - unexpected unlock net/core/sock.c:3117:6: sparse: sparse: context imbalance in 'lock_sock_fast' - different lock contexts for basic block net/core/sock.c:3611:13: sparse: sparse: context imbalance in 'proto_seq_start' - wrong count at exit net/core/sock.c:3623:13: sparse: sparse: context imbalance in 'proto_seq_stop' - wrong count at exit Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 2/2] net: socket: implement SO_DESCRIPTION
Hi Pascal,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/perf/core]
[also build test WARNING on linux/master v5.8]
[cannot apply to security/next-testing linus/master next-20200814]
[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]
url:
https://github.com/0day-ci/linux/commits/Pascal-Bouchareine/proc-socket-attach-description-to-sockets/20200816-090222
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
d903b6d029d66e6478562d75ea18d89098f7b7e8
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
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
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
In file included from arch/m68k/include/asm/io_mm.h:25,
from arch/m68k/include/asm/io.h:8,
from include/linux/scatterlist.h:9,
from include/linux/dma-mapping.h:11,
from include/linux/skbuff.h:31,
from include/linux/ip.h:16,
from include/net/ip.h:22,
from include/linux/errqueue.h:6,
from net/core/sock.c:91:
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsb':
arch/m68k/include/asm/raw_io.h:83:7: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
83 | ({u8 __w, __v = (b); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:430:3: note: in expansion of macro 'rom_out_8'
430 | rom_out_8(port, *buf++);
| ^
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw':
arch/m68k/include/asm/raw_io.h:86:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
86 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:448:3: note: in expansion of macro
'rom_out_be16'
448 | rom_out_be16(port, *buf++);
| ^~~~
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw_swapw':
arch/m68k/include/asm/raw_io.h:90:8: warning: variable '__w' set but not
used [-Wunused-but-set-variable]
90 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
|^~~
arch/m68k/include/asm/raw_io.h:466:3: note: in expansion of macro
'rom_out_le16'
466 | rom_out_le16(port, *buf++);
| ^~~~
In file included from include/linux/kernel.h:11,
from include/linux/unaligned/access_ok.h:5,
from arch/m68k/include/asm/unaligned.h:18,
from net/core/sock.c:88:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~
include/linux/scatterlist.h:143:10: note: in expansion of macro
'virt_addr_valid'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~~
In file included from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/skbuff.h:15,
from include/linux/ip.h:16,
from include/net/ip.h:22,
from include/linux/errqueue.h:6,
from net/core/sock.c:91:
include/linux/dma-mapping.h: In function 'dma_map_resource':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of
pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void
*)PAGE_OFFSET && (void *)(kaddr) < high_memory)
|
[PATCH 2/2] net: socket: implement SO_DESCRIPTION
This command attaches the zero terminated string in optval to the
socket for troubleshooting purposes. The free string is displayed in the
process fdinfo file for that fd (/proc//fdinfo/).
One intended usage is to allow processes to self-document sockets
for netstat and friends to report
We ignore optlen and constrain the string to a static max size
Signed-off-by: Pascal Bouchareine
---
include/net/sock.h| 4
include/uapi/asm-generic/socket.h | 2 ++
net/core/sock.c | 23 +++
net/socket.c | 5 +
4 files changed, 34 insertions(+)
diff --git a/include/net/sock.h b/include/net/sock.h
index 1183507df95b..6b4fd1383282 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -342,6 +342,7 @@ struct bpf_sk_storage;
*@sk_txtime_deadline_mode: set deadline mode for SO_TXTIME
*@sk_txtime_report_errors: set report errors mode for SO_TXTIME
*@sk_txtime_unused: unused txtime flags
+ *@sk_description: user supplied with SO_DESCRIPTION
*/
struct sock {
/*
@@ -519,6 +520,9 @@ struct sock {
struct bpf_sk_storage __rcu *sk_bpf_storage;
#endif
struct rcu_head sk_rcu;
+
+#defineSK_MAX_DESC_SIZE256
+ char*sk_description;
};
enum sk_pacing {
diff --git a/include/uapi/asm-generic/socket.h
b/include/uapi/asm-generic/socket.h
index 77f7c1638eb1..fb51c4bb7a12 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -119,6 +119,8 @@
#define SO_DETACH_REUSEPORT_BPF 68
+#define SO_DESCRIPTION 69
+
#if !defined(__KERNEL__)
#if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/net/core/sock.c b/net/core/sock.c
index 2e5b7870e5d3..2cb44a0e38b7 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -828,6 +828,24 @@ void sock_set_rcvbuf(struct sock *sk, int val)
}
EXPORT_SYMBOL(sock_set_rcvbuf);
+int sock_set_description(struct sock *sk, char __user *user_desc)
+{
+ char *old, *desc;
+
+ desc = strndup_user(user_desc, SK_MAX_DESC_SIZE, GFP_KERNEL_ACCOUNT);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ lock_sock(sk);
+ old = sk->sk_description;
+ sk->sk_description = desc;
+ release_sock(sk);
+
+ kfree(old);
+
+ return 0;
+}
+
/*
* This is meant for all protocols to use and covers goings on
* at the socket level. Everything here is generic.
@@ -850,6 +868,9 @@ int sock_setsockopt(struct socket *sock, int level, int
optname,
if (optname == SO_BINDTODEVICE)
return sock_setbindtodevice(sk, optval, optlen);
+ if (optname == SO_DESCRIPTION)
+ return sock_set_description(sk, optval);
+
if (optlen < sizeof(int))
return -EINVAL;
@@ -1792,6 +1813,8 @@ static void __sk_destruct(struct rcu_head *head)
RCU_INIT_POINTER(sk->sk_filter, NULL);
}
+ kfree(sk->sk_description);
+
sock_disable_timestamp(sk, SK_FLAGS_TIMESTAMP);
#ifdef CONFIG_BPF_SYSCALL
diff --git a/net/socket.c b/net/socket.c
index 976426d03f09..4f2c1a7744b0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -134,6 +134,11 @@ static void sock_show_fdinfo(struct seq_file *m, struct
file *f)
{
struct socket *sock = f->private_data;
+ lock_sock(sock->sk);
+ if (sock->sk->sk_description)
+ seq_printf(m, "desc:\t%s\n", sock->sk->sk_description);
+ release_sock(sock->sk);
+
if (sock->ops->show_fdinfo)
sock->ops->show_fdinfo(m, sock);
}
--
2.25.1
