* [PATCH][SMB3 client]
@ 2022-10-15 6:32 Steve French
2022-10-18 14:40 ` Tom Talpey
0 siblings, 1 reply; 2+ messages in thread
From: Steve French @ 2022-10-15 6:32 UTC (permalink / raw)
To: CIFS; +Cc: Paulo Alcantara, ronnie sahlberg
[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]
Change notification is a commonly supported feature by most servers,
and often used on other operating systems to detect when a file or
directory has changed and why, but the current ioctl to request
notification when a directory is changed does not return the
information about what changed (even though it is returned by
the server in the SMB3 change notify response), it simply returns
when there is a change.
This ioctl improves upon CIFS_IOC_NOTIFY by returning the notify
information structure which includes the name of the file(s) that
changed and why. See MS-SMB2 2.2.35 and MS-FSCC 2.7.1 for details
on the individual filter flags and the file_notify_information
structure returned.
To use this simply pass in the following (with enough space
to fit at least one file_notify_information structure)
struct __attribute__((__packed__)) smb3_notify {
uint32_t completion_filter;
bool watch_tree;
uint32_t data_len;
uint8_t data[];
} __packed;
using CIFS_IOC_NOTIFY_INFO 0xc009cf0b
or equivalently _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
The ioctl will block until the server detects a change to that
directory or its subdirectories (if watch_tree is set). See attached.
Also attached is a simple sample program to demonstrate its use. As an example
if you ran:
# ./new-inotify-ioc-test /mnt-on-client1/subdir
it will block until a change occurs. If you then do a
# touch /mnt-on-client2/subdir/newfile
you will see the following printed (in our sample program) showing the
notify information struct returned to the user
indicating the name of the file that was changed and what kind of change
notify completed. returned data size is 28
00000000: 00 00 00 00 03 00 00 00 0e 00 00 00 6e 00 65 00 ............n.e.
00000010: 77 00 66 00 69 00 6c 00 65 00 00 00 w.f.i.l.e...
--
Thanks,
Steve
[-- Attachment #2: new-inotify-ioc-test.c --]
[-- Type: text/x-csrc, Size: 2876 bytes --]
#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <stdbool.h>
#include <fcntl.h>
#include <string.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>
void dumpmem(FILE *out, const void *ptr, const size_t size)
{
const size_t BYTES_PER_LINE = 16;
size_t offset, read;
uint8_t *p = (uint8_t *)ptr;
const uint8_t *maxp = (p + size);
if (out == NULL || ptr == NULL || size == 0)
{
return;
}
for (offset = read = 0; offset != size; offset += read)
{
uint8_t buf[BYTES_PER_LINE];
for (read = 0; read != BYTES_PER_LINE && (&p[offset + read]) < maxp; read++)
{
buf[read] = p[offset + read];
}
if (read == 0)
return;
fprintf(out, "%.8x: ", (unsigned int)offset);
/* raw data */
for (size_t i = 0; i < read; i++)
{
fprintf(out, " %.2x", buf[i]);
if (BYTES_PER_LINE > 8 && BYTES_PER_LINE % 2 == 0 && i == (BYTES_PER_LINE / 2 - 1))
fprintf(out, " ");
}
/* ASCII */
if (read < BYTES_PER_LINE)
{
for (size_t i = read; i < BYTES_PER_LINE; i++)
{
fprintf(out, " ");
fprintf(out, " ");
if (BYTES_PER_LINE > 8 && BYTES_PER_LINE % 2 == 0 && i == (BYTES_PER_LINE / 2 - 1))
fprintf(out, " ");
}
}
fprintf(out, " ");
for (size_t i = 0; i < read; i++)
{
if (buf[i] <= 31 || buf[i] >= 127) /* ignore control and non-ASCII characters */
fprintf(out, ".");
else
fprintf(out, "%c", buf[i]);
}
fprintf(out, "\n");
}
}
/* See MS-SMB2 2.2.35 for a definition of the individual filter flags */
struct __attribute__((__packed__)) smb3_notify {
uint32_t completion_filter;
bool watch_tree;
uint32_t data_len;
uint8_t data[];
} __packed;
#define CIFS_IOC_NOTIFY 0x4005cf09 /* previous ioctl which simply returns when changes occur */
#define CIFS_IOC_NOTIFY_INFO 0xc009cf0b /* new ioctl for change notification */
int main(int argc, char **argv)
{
struct smb3_notify *pnotify;
int f, g;
if ((f = open(argv[1], O_RDONLY)) < 0) {
fprintf(stderr, "Failed to open %s\n", argv[1]);
exit(1);
}
pnotify = malloc(sizeof(struct smb3_notify) + 200);
memset(pnotify, 0, sizeof(struct smb3_notify) + 200);
pnotify->watch_tree = false;
pnotify->completion_filter = 0xFFF;
pnotify->data_len = 200;
if (ioctl(f, CIFS_IOC_NOTIFY_INFO, pnotify) < 0)
printf("Error %d returned from ioctl\n", errno);
else {
printf("notify completed. returned data size is %d\n", pnotify->data_len);
dumpmem(stdout, pnotify->data, pnotify->data_len);
}
}
[-- Attachment #3: 0001-smb3-improve-SMB3-change-notification-support.patch --]
[-- Type: text/x-patch, Size: 9991 bytes --]
From f5dd7834b2c3468a1e83c1747299acd160d2132c Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sat, 15 Oct 2022 00:43:22 -0500
Subject: [PATCH] smb3: improve SMB3 change notification support
Change notification is a commonly supported feature by most servers,
but the current ioctl to request notification when a directory is
changed does not return the information about what changed
(even though it is returned by the server in the SMB3 change
notify response), it simply returns when there is a change.
This ioctl improves upon CIFS_IOC_NOTIFY by returning the notify
information structure which includes the name of the file(s) that
changed and why. See MS-SMB2 2.2.35 for details on the individual
filter flags and the file_notify_information struture returned.
To use this simply pass in the following (with enough space
to fit at least one file_notify_information structure)
struct __attribute__((__packed__)) smb3_notify {
uint32_t completion_filter;
bool watch_tree;
uint32_t data_len;
uint8_t data[];
} __packed;
using CIFS_IOC_NOTIFY_INFO 0xc009cf0b
or equivalently _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
The ioctl will block until the server detects a change to that
directory or its subdirectories (if watch_tree is set).
Acked-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/cifs/cifs_ioctl.h | 8 ++++++++
fs/cifs/cifsglob.h | 2 +-
fs/cifs/ioctl.c | 25 ++++++++++++++++++++++++-
fs/cifs/smb2ops.c | 35 ++++++++++++++++++++++++++++-------
fs/cifs/smb2pdu.c | 30 +++++++++++++++++++++++++++---
fs/cifs/smb2proto.h | 3 ++-
6 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/fs/cifs/cifs_ioctl.h b/fs/cifs/cifs_ioctl.h
index b87cbbe6d2d4..d86d78d5bfdc 100644
--- a/fs/cifs/cifs_ioctl.h
+++ b/fs/cifs/cifs_ioctl.h
@@ -91,6 +91,13 @@ struct smb3_notify {
bool watch_tree;
} __packed;
+struct smb3_notify_info {
+ __u32 completion_filter;
+ bool watch_tree;
+ __u32 data_len; /* size of notify data below */
+ __u8 notify_data[];
+} __packed;
+
#define CIFS_IOCTL_MAGIC 0xCF
#define CIFS_IOC_COPYCHUNK_FILE _IOW(CIFS_IOCTL_MAGIC, 3, int)
#define CIFS_IOC_SET_INTEGRITY _IO(CIFS_IOCTL_MAGIC, 4)
@@ -100,6 +107,7 @@ struct smb3_notify {
#define CIFS_DUMP_KEY _IOWR(CIFS_IOCTL_MAGIC, 8, struct smb3_key_debug_info)
#define CIFS_IOC_NOTIFY _IOW(CIFS_IOCTL_MAGIC, 9, struct smb3_notify)
#define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info)
+#define CIFS_IOC_NOTIFY_INFO _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
#define CIFS_IOC_SHUTDOWN _IOR ('X', 125, __u32)
/*
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 9c0253835f1c..1420acf987f0 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -454,7 +454,7 @@ struct smb_version_operations {
int (*enum_snapshots)(const unsigned int xid, struct cifs_tcon *tcon,
struct cifsFileInfo *src_file, void __user *);
int (*notify)(const unsigned int xid, struct file *pfile,
- void __user *pbuf);
+ void __user *pbuf, bool return_changes);
int (*query_mf_symlink)(unsigned int, struct cifs_tcon *,
struct cifs_sb_info *, const unsigned char *,
char *, unsigned int *);
diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
index b6e6e5d6c8dd..89d5fa887364 100644
--- a/fs/cifs/ioctl.c
+++ b/fs/cifs/ioctl.c
@@ -484,12 +484,35 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
tcon = tlink_tcon(tlink);
if (tcon && tcon->ses->server->ops->notify) {
rc = tcon->ses->server->ops->notify(xid,
- filep, (void __user *)arg);
+ filep, (void __user *)arg,
+ false /* no ret data */);
cifs_dbg(FYI, "ioctl notify rc %d\n", rc);
} else
rc = -EOPNOTSUPP;
cifs_put_tlink(tlink);
break;
+ case CIFS_IOC_NOTIFY_INFO:
+ if (!S_ISDIR(inode->i_mode)) {
+ /* Notify can only be done on directories */
+ rc = -EOPNOTSUPP;
+ break;
+ }
+ cifs_sb = CIFS_SB(inode->i_sb);
+ tlink = cifs_sb_tlink(cifs_sb);
+ if (IS_ERR(tlink)) {
+ rc = PTR_ERR(tlink);
+ break;
+ }
+ tcon = tlink_tcon(tlink);
+ if (tcon && tcon->ses->server->ops->notify) {
+ rc = tcon->ses->server->ops->notify(xid,
+ filep, (void __user *)arg,
+ true /* return details */);
+ cifs_dbg(FYI, "ioctl notify info rc %d\n", rc);
+ } else
+ rc = -EOPNOTSUPP;
+ cifs_put_tlink(tlink);
+ break;
case CIFS_IOC_SHUTDOWN:
rc = cifs_shutdown(inode->i_sb, arg);
break;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index b907d1fab8d9..17b25153cb68 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2018,9 +2018,10 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
static int
smb3_notify(const unsigned int xid, struct file *pfile,
- void __user *ioc_buf)
+ void __user *ioc_buf, bool return_changes)
{
- struct smb3_notify notify;
+ struct smb3_notify_info notify;
+ struct smb3_notify_info __user *pnotify_buf;
struct dentry *dentry = pfile->f_path.dentry;
struct inode *inode = file_inode(pfile);
struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
@@ -2028,10 +2029,12 @@ smb3_notify(const unsigned int xid, struct file *pfile,
struct cifs_fid fid;
struct cifs_tcon *tcon;
const unsigned char *path;
+ char *returned_ioctl_info = NULL;
void *page = alloc_dentry_path();
__le16 *utf16_path = NULL;
u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
int rc = 0;
+ __u32 ret_len = 0;
path = build_path_from_dentry(dentry, page);
if (IS_ERR(path)) {
@@ -2045,9 +2048,17 @@ smb3_notify(const unsigned int xid, struct file *pfile,
goto notify_exit;
}
- if (copy_from_user(¬ify, ioc_buf, sizeof(struct smb3_notify))) {
- rc = -EFAULT;
- goto notify_exit;
+ if (return_changes) {
+ if (copy_from_user(¬ify, ioc_buf, sizeof(struct smb3_notify_info))) {
+ rc = -EFAULT;
+ goto notify_exit;
+ }
+ } else {
+ if (copy_from_user(¬ify, ioc_buf, sizeof(struct smb3_notify))) {
+ rc = -EFAULT;
+ goto notify_exit;
+ }
+ notify.data_len = 0;
}
tcon = cifs_sb_master_tcon(cifs_sb);
@@ -2064,12 +2075,22 @@ smb3_notify(const unsigned int xid, struct file *pfile,
goto notify_exit;
rc = SMB2_change_notify(xid, tcon, fid.persistent_fid, fid.volatile_fid,
- notify.watch_tree, notify.completion_filter);
+ notify.watch_tree, notify.completion_filter,
+ notify.data_len, &returned_ioctl_info, &ret_len);
SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid);
cifs_dbg(FYI, "change notify for path %s rc %d\n", path, rc);
-
+ if (return_changes && (ret_len > 0) && (notify.data_len > 0)) {
+ if (ret_len > notify.data_len)
+ ret_len = notify.data_len;
+ pnotify_buf = (struct smb3_notify_info __user *)ioc_buf;
+ if (copy_to_user(pnotify_buf->notify_data, returned_ioctl_info, ret_len))
+ rc = -EFAULT;
+ else if (copy_to_user(&pnotify_buf->data_len, &ret_len, sizeof(ret_len)))
+ rc = -EFAULT;
+ }
+ kfree(returned_ioctl_info);
notify_exit:
free_dentry_path(page);
kfree(utf16_path);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index f8f89ff96c5d..d5cf012bfc10 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3710,11 +3710,13 @@ SMB2_notify_init(const unsigned int xid, struct smb_rqst *rqst,
int
SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_fid, u64 volatile_fid, bool watch_tree,
- u32 completion_filter)
+ u32 completion_filter, u32 max_out_data_len, char **out_data,
+ u32 *plen /* returned data len */)
{
struct cifs_ses *ses = tcon->ses;
struct TCP_Server_Info *server = cifs_pick_channel(ses);
struct smb_rqst rqst;
+ struct smb2_change_notify_rsp *smb_rsp;
struct kvec iov[1];
struct kvec rsp_iov = {NULL, 0};
int resp_buftype = CIFS_NO_BUFFER;
@@ -3730,6 +3732,9 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
memset(&rqst, 0, sizeof(struct smb_rqst));
memset(&iov, 0, sizeof(iov));
+ if (plen)
+ *plen = 0;
+
rqst.rq_iov = iov;
rqst.rq_nvec = 1;
@@ -3748,9 +3753,28 @@ SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
cifs_stats_fail_inc(tcon, SMB2_CHANGE_NOTIFY_HE);
trace_smb3_notify_err(xid, persistent_fid, tcon->tid, ses->Suid,
(u8)watch_tree, completion_filter, rc);
- } else
+ } else {
trace_smb3_notify_done(xid, persistent_fid, tcon->tid,
- ses->Suid, (u8)watch_tree, completion_filter);
+ ses->Suid, (u8)watch_tree, completion_filter);
+ /* validate that notify information is plausible */
+ if ((rsp_iov.iov_base == NULL) ||
+ (rsp_iov.iov_len < sizeof(struct smb2_change_notify_rsp)))
+ goto cnotify_exit;
+
+ smb_rsp = (struct smb2_change_notify_rsp *)rsp_iov.iov_base;
+
+ smb2_validate_iov(le16_to_cpu(smb_rsp->OutputBufferOffset),
+ le32_to_cpu(smb_rsp->OutputBufferLength), &rsp_iov,
+ sizeof(struct file_notify_information));
+
+ *out_data = kmemdup((char *)smb_rsp + le16_to_cpu(smb_rsp->OutputBufferOffset),
+ le32_to_cpu(smb_rsp->OutputBufferLength), GFP_KERNEL);
+ if (*out_data == NULL) {
+ rc = -ENOMEM;
+ goto cnotify_exit;
+ } else
+ *plen = le32_to_cpu(smb_rsp->OutputBufferLength);
+ }
cnotify_exit:
if (rqst.rq_iov)
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 7818d0b83567..be21b5d26f67 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -144,7 +144,8 @@ extern int SMB2_ioctl_init(struct cifs_tcon *tcon,
extern void SMB2_ioctl_free(struct smb_rqst *rqst);
extern int SMB2_change_notify(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_fid, u64 volatile_fid, bool watch_tree,
- u32 completion_filter);
+ u32 completion_filter, u32 max_out_data_len,
+ char **out_data, u32 *plen /* returned data len */);
extern int __SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_fid, u64 volatile_fid,
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH][SMB3 client]
2022-10-15 6:32 [PATCH][SMB3 client] Steve French
@ 2022-10-18 14:40 ` Tom Talpey
0 siblings, 0 replies; 2+ messages in thread
From: Tom Talpey @ 2022-10-18 14:40 UTC (permalink / raw)
To: Steve French, CIFS; +Cc: Paulo Alcantara, ronnie sahlberg
On 10/15/2022 2:32 AM, Steve French wrote:
> Change notification is a commonly supported feature by most servers,
> and often used on other operating systems to detect when a file or
> directory has changed and why, but the current ioctl to request
> notification when a directory is changed does not return the
> information about what changed (even though it is returned by
> the server in the SMB3 change notify response), it simply returns
> when there is a change.
It's a useful enhancement to the API to see the details of the
notification. What Linux applications might use it, was this
requested by soneone?
I do have one general comment on the implementation. I am pasting
some snippets from the patch you attached.
The new structure adds two fields:
+struct smb3_notify_info {
+ __u32 completion_filter;
+ bool watch_tree;
+ __u32 data_len; /* size of notify data below */
+ __u8 notify_data[];
+} __packed;
+
however the vector adds a single boolean:
int (*notify)(const unsigned int xid, struct file *pfile,
- void __user *pbuf);
+ void __user *pbuf, bool return_changes);
Why a boolean? Wouldn't it be more logical, and general, to pass
the length instead? If zero, it's the existing behavior, and if
set to a length, then that many bytes max are requested?
It would also be logical to return the actual number of bytes returned,
so perhaps passing the length by reference.
int (*notify)(const unsigned int xid, struct file *pfile,
- void __user *pbuf);
+ void __user *pbuf, int *length);
Finally, both the existing smb3_notify and the new smb3_notify_info
use a strange combination of native and size-specific types, packed
into a struct as if they were required to be wire-encoded. The __u32
packed with a bool, in both cases, and the __u32 length in the new
one. The bool could be anything, how does the "packed" attribute
actually work? Any why the inflexible __u32's?
struct smb3_notify {
__u32 completion_filter;
bool watch_tree;
} __packed;
It seems to me that these structs should be defined in base
types and marshaled/unmarshaled in the smb3_notify processing.
Of course, the existing smb3_notify maybe needs to remain for
binary compatibility, but the new structure should not copy and
extend it.
Tom.
> This ioctl improves upon CIFS_IOC_NOTIFY by returning the notify
> information structure which includes the name of the file(s) that
> changed and why. See MS-SMB2 2.2.35 and MS-FSCC 2.7.1 for details
> on the individual filter flags and the file_notify_information
> structure returned.
>
> To use this simply pass in the following (with enough space
> to fit at least one file_notify_information structure)
>
> struct __attribute__((__packed__)) smb3_notify {
> uint32_t completion_filter;
> bool watch_tree;
> uint32_t data_len;
> uint8_t data[];
> } __packed;
>
> using CIFS_IOC_NOTIFY_INFO 0xc009cf0b
> or equivalently _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
>
> The ioctl will block until the server detects a change to that
> directory or its subdirectories (if watch_tree is set). See attached.
>
> Also attached is a simple sample program to demonstrate its use. As an example
> if you ran:
> # ./new-inotify-ioc-test /mnt-on-client1/subdir
> it will block until a change occurs. If you then do a
> # touch /mnt-on-client2/subdir/newfile
> you will see the following printed (in our sample program) showing the
> notify information struct returned to the user
> indicating the name of the file that was changed and what kind of change
> notify completed. returned data size is 28
> 00000000: 00 00 00 00 03 00 00 00 0e 00 00 00 6e 00 65 00 ............n.e.
> 00000010: 77 00 66 00 69 00 6c 00 65 00 00 00 w.f.i.l.e...
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-18 14:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-15 6:32 [PATCH][SMB3 client] Steve French
2022-10-18 14:40 ` Tom Talpey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).