net/unix_diag: Provide UDIAG_SHOW_VFS2 attribute to fetch complete inode number

Currently unix_diag_vfs structure reports unix socket inode as u32 value which of course doesn't fit to ino_t type and

the number may be trimmed. Lets rather deprecate old UDIAG_SHOW_VFS interface and provide UDIAG_SHOW_VFS2 (with one field "__zero" reserved which we could extend in future).

CC Andrey Vagin <[email protected]>

CC David S. Miller <[email protected]>

CC Pavel Emelyanov <[email protected]>

Signed-off-by Cyrill Gorcunov <[email protected]>

---

I build-tested it only thus not for inclusion yet, but rather to discuss if there some better way to handle this potential problem.

 include/uapi/linux/unix_diag.h |    8 ++++++++
 net/unix/diag.c                |   25 ++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

Index: linux-ml.git/include/uapi/linux/unix_diag.h
===================================================================

include/uapi/linux/unix_diag.h +2 -0

@@ -19,6 +19,7 @@ struct unix_diag_req {
 #define UDIAG_SHOW_ICONS	0x00000008	/* show pending connections */
 #define UDIAG_SHOW_RQLEN	0x00000010	/* show skb receive queue len */
 #define UDIAG_SHOW_MEMINFO	0x00000020	/* show memory info of a socket */
+#define UDIAG_SHOW_VFS2		0x00000040	/* show VFS inode info v2 */
 
 struct unix_diag_msg {
 	__u8	udiag_family;

@@ -39,6 +40,7 @@ enum {
 	UNIX_DIAG_RQLEN,
 	UNIX_DIAG_MEMINFO,
 	UNIX_DIAG_SHUTDOWN,
+	UNIX_DIAG_VFS2,
 
 	__UNIX_DIAG_MAX,
 };

include/uapi/linux/unix_diag.h +2 -0

+};
+
 struct unix_diag_rqlen {
 	__u32	udiag_rqueue;
 	__u32	udiag_wqueue;

Index: linux-ml.git/net/unix/diag.c
===================================================================

net/unix/diag.c +16 -9

@@ -19,17 +19,24 @@ static int sk_diag_dump_name(struct sock
 		       addr->name->sun_path);
 }
 
-static int sk_diag_dump_vfs(struct sock *sk, struct sk_buff *nlskb)
+static int sk_diag_dump_vfs(struct sock *sk, struct sk_buff *nlskb, unsigned int flags)
 {
 	struct dentry *dentry = unix_sk(sk)->path.dentry;
 
 	if (dentry) {
-		struct unix_diag_vfs uv = {
-			.udiag_vfs_ino = d_backing_inode(dentry)->i_ino,
-			.udiag_vfs_dev = dentry->d_sb->s_dev,
-		};
-
-		return nla_put(nlskb, UNIX_DIAG_VFS, sizeof(uv), &uv);
+		if (flags & UDIAG_SHOW_VFS2) {
+			struct unix_diag_vfs uv = {
+				.udiag_vfs_ino = d_backing_inode(dentry)->i_ino,
+				.udiag_vfs_dev = dentry->d_sb->s_dev,
+			};
+			return nla_put(nlskb, UNIX_DIAG_VFS, sizeof(uv), &uv);
+		} else {
+			struct unix_diag_vfs2 uv = {
+				.udiag_vfs_ino = d_backing_inode(dentry)->i_ino,
+				.udiag_vfs_dev = dentry->d_sb->s_dev,
+			};
+			return nla_put(nlskb, UDIAG_SHOW_VFS2, sizeof(uv), &uv);
+		}
 	}
 
 	return 0;

@@ -132,8 +139,8 @@ static int sk_diag_fill(struct sock *sk,
 	    sk_diag_dump_name(sk, skb))
 		goto out_nlmsg_trim;
 
-	if ((req->udiag_show & UDIAG_SHOW_VFS) &&
-	    sk_diag_dump_vfs(sk, skb))
+	if ((req->udiag_show & (UDIAG_SHOW_VFS | UDIAG_SHOW_VFS2)) &&
+	    sk_diag_dump_vfs(sk, skb, req->udiag_show))
 		goto out_nlmsg_trim;
 
 	if ((req->udiag_show & UDIAG_SHOW_PEER) &&

Quoted 2 lines · Cyrill Gorcunov

Currently unix_diag_vfs structure reports unix socket inode as u32 value which of course doesn't fit to ino_t type and

BTW: As far as I understand, it is not a problem right now, because get_next_ino returns int. And I'm agree that it maybe a problem in a future and it is better to be ready.

Quoted 3 lines

the number may be trimmed. Lets rather deprecate old UDIAG_SHOW_VFS interface and provide UDIAG_SHOW_VFS2 (with one field "__zero" reserved which we could extend in future).

There is one more place where we return ino as u32:

static int sk_diag_dump_peer(struct sock *sk, struct sk_buff *nlskb)
....

                return nla_put_u32(nlskb, UNIX_DIAG_PEER, ino);

include/uapi/linux/unix_diag.h struct unix_diag_vfs { · line 58

Quoted 4 lines

include/uapi/linux/unix_diag.h +4 -0

+struct unix_diag_vfs2 {
+	__u64	udiag_vfs_ino;
+	__u32	udiag_vfs_dev;
+	__u32	__zero;		/* Reserve for future use */

How can a user understand whether this field is used or not?

Each netlink attribute has its size in a header. Any attribute can be extended, and users can understand which fields are filled by a size of an attribute.

Currently unix_diag_vfs structure reports unix socket inode as u32 value which of course doesn't fit to ino_t type and the number may be trimmed. Lets rather deprecate old UDIAG_SHOW_VFS interface and provide UDIAG_SHOW_VFS2 (with one field "__zero" reserved which we could extend in future).

net/unix/diag.c +2 -7

-static int sk_diag_dump_vfs(struct sock *sk, struct sk_buff *nlskb)
+static int sk_diag_dump_vfs(struct sock *sk, struct sk_buff *nlskb, unsigned int flags)
 {
 	struct dentry *dentry = unix_sk(sk)->path.dentry;
 
 	if (dentry) {
-		struct unix_diag_vfs uv = {
-			.udiag_vfs_ino = d_backing_inode(dentry)->i_ino,
-			.udiag_vfs_dev = dentry->d_sb->s_dev,
-		};
-
-		return nla_put(nlskb, UNIX_DIAG_VFS, sizeof(uv), &uv);
+		if (flags & UDIAG_SHOW_VFS2) {

I think __zero should be explicitly set to 0.