Re: [PATCH v2] cifs: remove unnecessary copies of tcon->crfid.fid
I changed the comment to + /* +* See commit 2f94a3125b87. Increment the refcount when we +* get a lease for root, release it if lease break occurs +*/ and added Aurelien's Reviewed-by. Let me know if you see any additional problems. On Sat, Apr 17, 2021 at 5:54 AM Aurélien Aptel wrote: > > Hi, > > This is better I think. > > Muhammad Usama Anjum writes: > > @@ -894,6 +891,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon > > *tcon, > > > > /* BB TBD check to see if oplock level check can be removed below */ > > if (o_rsp->OplockLevel == SMB2_OPLOCK_LEVEL_LEASE) { > > + /* > > + * caller expects this func to set the fid in crfid to valid > > + * cached root, so increment the refcount. > > + */ > > This comment is misleading. crfid variable doesn't exist anymore, and > the kref_get() here is because of this commit: > > commit 2f94a3125b87 > Author: Ronnie Sahlberg > Date: Thu Mar 28 11:20:02 2019 +1000 > > cifs: fix kref underflow in close_shroot() > > [...] > --> This extra get() is only used to hold the structure until we get a > lease > --> break from the server at which point we will kref_put() it during > lease > --> processing. > [...] > > > > When we queue a lease break, we usually get() the cifsFileInfo, but if > that cifsFileInfo is backed by a cached_fid, the cached_fid isn't > bumped. That commit was probably a work around for that. > > @Ronnie : > > struct cached_fid is starting to look very much like struct > cifsFileInfo. I wonder why we couldn't use it, along with > find_writable_file()/find_readable_file() to handle the caching. > > Alternatively, make cifsFileInfo use cached_fid (perhaps renaming it in > the process, I don't know) > > Because I suspect a lot more issues will come up regarding cached_fid > refcount and cifsFileInfo refcount going out of sync otherwise. > > Cheers, > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) > -- Thanks, Steve
Re: [PATCH] fs: cifs: Fix spelling of 'security'
Merged into cifs-2.6.git for-next (strangely ... this patch was sent to my spam folder in gmail so didn't notice it until today, and by accident). On Wed, Apr 7, 2021 at 9:03 AM wrote: > > From: "jack1.li_cp" > > secuirty -> security > > Signed-off-by: jack1.li_cp > --- > fs/cifs/cifsacl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > index 562913e..d2d8e26 100644 > --- a/fs/cifs/cifsacl.c > +++ b/fs/cifs/cifsacl.c > @@ -1418,7 +1418,7 @@ int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, > * Add three ACEs for owner, group, everyone getting rid of other ACEs > * as chmod disables ACEs and set the security descriptor. Allocate > * memory for the smb header, set security descriptor request security > -* descriptor parameters, and secuirty descriptor itself > +* descriptor parameters, and security descriptor itself > */ > secdesclen = max_t(u32, secdesclen, DEFAULT_SEC_DESC_LEN); > pnntsd = kmalloc(secdesclen, GFP_KERNEL); > -- > 1.9.1 > > -- Thanks, Steve
Re: [PATCH][next] cifs: cifspdu.h: Replace one-element array with flexible-array member
merged into cifs-2.6.git for-next On Fri, Mar 26, 2021 at 12:02 PM Gustavo A. R. Silva wrote: > > > > On 3/26/21 10:54, Aurélien Aptel wrote: > > "Gustavo A. R. Silva" writes: > >> There is a regular need in the kernel to provide a way to declare having > >> a dynamically sized set of trailing elements in a structure. Kernel code > >> should always use “flexible array members”[1] for these cases. The older > >> style of one-element or zero-length arrays should no longer be used[2]. > > > > I've checked the usages of the struct, looks OK (we don't allocate it > > directly, we use memory from the small/big buff pools). > > Awesome. :) > > > Reviewed-by: Aurelien Aptel > > Thank you, Aurelien. > -- > Gustavo -- Thanks, Steve
Re: [PATCH] cifs: Remove useless variable
merged into cifs-2.6.git for-next On Thu, Apr 8, 2021 at 3:33 AM Jiapeng Chong wrote: > > Fix the following gcc warning: > > fs/cifs/cifsacl.c:1097:8: warning: variable ‘nmode’ set but not used > [-Wunused-but-set-variable]. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > fs/cifs/cifsacl.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > index d178cf8..fdb258a 100644 > --- a/fs/cifs/cifsacl.c > +++ b/fs/cifs/cifsacl.c > @@ -1094,11 +1094,9 @@ static int set_chmod_dacl(struct cifs_acl *pdacl, > struct cifs_acl *pndacl, > struct cifs_ace *pnntace = NULL; > char *nacl_base = NULL; > u32 num_aces = 0; > - __u64 nmode; > bool new_aces_set = false; > > /* Assuming that pndacl and pnmode are never NULL */ > - nmode = *pnmode; > nacl_base = (char *)pndacl; > nsize = sizeof(struct cifs_acl); > > -- > 1.8.3.1 > -- Thanks, Steve
Re: [PATCH] fs: cifs: Remove repeated struct declaration
merged into cifs-2.6.git for-next On Thu, Apr 8, 2021 at 9:47 PM Wan Jiabing wrote: > > struct cifs_writedata is declared twice. > One is declared at 209th line. > And struct cifs_writedata is defined blew. > The declaration hear is not needed. Remove the duplicate. > > Signed-off-by: Wan Jiabing > --- > fs/cifs/cifsglob.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index ec824ab8c5ca..5ec60745034e 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1316,8 +1316,6 @@ struct cifs_readdata { > struct page **pages; > }; > > -struct cifs_writedata; > - > /* asynchronous write support */ > struct cifs_writedata { > struct kref refcount; > -- > 2.25.1 > -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit e49d033bddf5b565044e2abe4241353959bc9120: Linux 5.12-rc6 (2021-04-04 14:15:36 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-rc6-smb3 for you to fetch changes up to 0fc9322ab5e1fe6910c9673e1a7ff29f7dd72611: cifs: escape spaces in share names (2021-04-07 21:30:27 -0500) 3 cifs/smb3 fixes, 2 for stable, includes a reconnect fix (for case when server address changed) and fix for proper display of devnames (when have space or tab). Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/550 Maciek Borzecki (1): cifs: escape spaces in share names Shyam Prasad N (1): cifs: On cifs_reconnect, resolve the hostname again. Wan Jiabing (1): fs: cifs: Remove unnecessary struct declaration fs/cifs/Kconfig| 3 +-- fs/cifs/Makefile | 5 +++-- fs/cifs/cifsfs.c | 3 ++- fs/cifs/cifsglob.h | 2 -- fs/cifs/connect.c | 17 - 5 files changed, 22 insertions(+), 8 deletions(-) -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed - (a tmp branch was accidentally pushed) On Thu, Apr 8, 2021 at 7:07 AM Stephen Rothwell wrote: > > Hi all, > > Commit > > e67fcb31fb0e ("stuff") > > is missing a Signed-off-by from its author and comitter. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: [PATCH] cifs: escape spaces in share names
Good catch - added one other minor thing, whitespace in share name could be a space or a tab, so changed that line to: + seq_escape(m, devname, " \t"); from + seq_escape(m, devname, " "); On Wed, Apr 7, 2021 at 12:35 AM Maciek Borzecki wrote: > > Commit 653a5efb849a ("cifs: update super_operations to show_devname") > introduced the display of devname for cifs mounts. However, when mounting > a share which has a whitespace in the name, that exact share name is also > displayed in mountinfo. Make sure that all whitespace is escaped. > > Signed-off-by: Maciek Borzecki > --- > fs/cifs/cifsfs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index > 099ad9f3660bb28db1b6a9aea9538282b41c6455..3c6cb85b95e207df48f10cc9df937cdda24e > 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -476,7 +476,8 @@ static int cifs_show_devname(struct seq_file *m, struct > dentry *root) > seq_puts(m, "none"); > else { > convert_delimiter(devname, '/'); > - seq_puts(m, devname); > + /* escape all spaces in share names */ > + seq_escape(m, devname, " "); > kfree(devname); > } > return 0; > -- > 2.31.1 > -- Thanks, Steve From 1ee4f33ffa830267d324b0d547facd25dd683a12 Mon Sep 17 00:00:00 2001 From: Maciek Borzecki Date: Tue, 6 Apr 2021 17:02:29 +0200 Subject: [PATCH] cifs: escape spaces in share names Commit 653a5efb849a ("cifs: update super_operations to show_devname") introduced the display of devname for cifs mounts. However, when mounting a share which has a whitespace in the name, that exact share name is also displayed in mountinfo. Make sure that all whitespace is escaped. Signed-off-by: Maciek Borzecki CC: # 5.11+ Reviewed-by: Shyam Prasad N Signed-off-by: Steve French --- fs/cifs/cifsfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 099ad9f3660b..3c6cb85b95e2 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -476,7 +476,8 @@ static int cifs_show_devname(struct seq_file *m, struct dentry *root) seq_puts(m, "none"); else { convert_delimiter(devname, '/'); - seq_puts(m, devname); + /* escape all spaces in share names */ + seq_escape(m, devname, " \t"); kfree(devname); } return 0; -- 2.27.0
Re: [PATCH] fs: cifs: Remove unnecessary struct declaration
merged into cifs-2.6.git for-next On Thu, Apr 1, 2021 at 2:52 AM Wan Jiabing wrote: > > struct cifs_readdata is declared twice. One is declared > at 208th line. > And struct cifs_readdata is defined blew. > The declaration here is not needed. Remove the duplicate. > > Signed-off-by: Wan Jiabing > --- > fs/cifs/cifsglob.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 67c056a9a519..ec824ab8c5ca 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -1283,8 +1283,6 @@ struct cifs_aio_ctx { > booldirect_io; > }; > > -struct cifs_readdata; > - > /* asynchronous read support */ > struct cifs_readdata { > struct kref refcount; > -- > 2.25.1 > -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit 0d02ec6b3136c73c09e7859f0d0e4e2c4c07b49b: Linux 5.12-rc4 (2021-03-21 14:56:43 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-rc4-smb3 for you to fetch changes up to cfc63fc8126a93cbf95379bc4cad79a7b15b6ece: smb3: fix cached file size problems in duplicate extents (reflink) (2021-03-26 18:41:55 -0500) 5 cifs/smb3 fixes, 2 for stable, includes an important fix for encryption and an ACL fix, as well as two fixes for possible data corruptions (one for reflink and one for SMB1) Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/543 Ronnie Sahlberg (1): cifs: revalidate mapping when we open files for SMB1 POSIX Shyam Prasad N (2): cifs: Adjust key sizes and key generation routines for AES256 encryption cifs: Fix chmod with modefromsid when an older ACE already exists. Steve French (1): smb3: fix cached file size problems in duplicate extents (reflink) Vincent Whitchurch (1): cifs: Silently ignore unknown oplock break handle fs/cifs/cifsacl.c | 3 +-- fs/cifs/cifsglob.h | 4 ++-- fs/cifs/cifspdu.h | 5 + fs/cifs/file.c | 1 + fs/cifs/smb2glob.h | 1 + fs/cifs/smb2misc.c | 4 ++-- fs/cifs/smb2ops.c | 27 --- fs/cifs/smb2transport.c | 37 - 8 files changed, 60 insertions(+), 22 deletions(-) -- Thanks, Steve
Re: [PATCH v3] cifs: Silently ignore unknown oplock break handle
merged into cifs-2.6.git for-next On Fri, Mar 19, 2021 at 9:06 AM Tom Talpey via samba-technical wrote: > > LGTM feel free to add > > Reviewed-By: Tom Talpey > > On 3/19/2021 9:57 AM, Vincent Whitchurch wrote: > > Make SMB2 not print out an error when an oplock break is received for an > > unknown handle, similar to SMB1. The debug message which is printed for > > these unknown handles may also be misleading, so fix that too. > > > > The SMB2 lease break path is not affected by this patch. > > > > Without this, a program which writes to a file from one thread, and > > opens, reads, and writes the same file from another thread triggers the > > below errors several times a minute when run against a Samba server > > configured with "smb2 leases = no". > > > > CIFS: VFS: \\192.168.0.1 No task to wake, unknown frame received! NumMids > > 2 > > : 424d53fe 0040 0012 .SMB@... > > 0010: 0001 > > 0020: > > 0030: > > > > Signed-off-by: Vincent Whitchurch > > --- > > > > Notes: > > v3: > > - Change debug print to Tom Talpey's suggestion > > > > v2: > > - Drop change to lease break > > - Rewrite commit message > > > > fs/cifs/smb2misc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > > index 60d4bd1eae2b..76cd05b8d53b 100644 > > --- a/fs/cifs/smb2misc.c > > +++ b/fs/cifs/smb2misc.c > > @@ -754,8 +754,8 @@ smb2_is_valid_oplock_break(char *buffer, struct > > TCP_Server_Info *server) > > } > > } > > spin_unlock(_tcp_ses_lock); > > - cifs_dbg(FYI, "Can not process oplock break for non-existent > > connection\n"); > > - return false; > > + cifs_dbg(FYI, "No file id matched, oplock break ignored\n"); > > + return true; > > } > > > > void > > > -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit 1e28eed17697bcf343c6743f0028cc3b5dd88bf0: Linux 5.12-rc3 (2021-03-14 14:41:02 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-rc3-smb3 for you to fetch changes up to 65af8f0166f4d15e61c63db498ec7981acdd897f: cifs: fix allocation size on newly created files (2021-03-19 11:51:31 -0500) 5 cifs/smb3 fixes, 3 for stable, including an important ACL fix and security signature fix Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/530 Aurelien Aptel (1): cifs: warn and fail if trying to use rootfs without the config option Liu xuzhi (1): fs/cifs/: fix misspellings using codespell tool Shyam Prasad N (1): cifs: update new ACE pointer after populate_new_aces. Steve French (1): cifs: fix allocation size on newly created files Vincent Whitchurch (1): cifs: Fix preauth hash corruption fs/cifs/cifs_swn.c | 2 +- fs/cifs/cifsacl.c| 9 ++--- fs/cifs/fs_context.c | 6 -- fs/cifs/inode.c | 10 +- fs/cifs/transport.c | 7 ++- 5 files changed, 26 insertions(+), 8 deletions(-) -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit 1e28eed17697bcf343c6743f0028cc3b5dd88bf0: Linux 5.12-rc3 (2021-03-14 14:41:02 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-rc3-smb3 for you to fetch changes up to 65af8f0166f4d15e61c63db498ec7981acdd897f: cifs: fix allocation size on newly created files (2021-03-19 11:51:31 -0500) 5 cifs/smb3 fixes, 3 for stable, including an important ACL fix and security signature fix Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/530 Aurelien Aptel (1): cifs: warn and fail if trying to use rootfs without the config option Liu xuzhi (1): fs/cifs/: fix misspellings using codespell tool Shyam Prasad N (1): cifs: update new ACE pointer after populate_new_aces. Steve French (1): cifs: fix allocation size on newly created files Vincent Whitchurch (1): cifs: Fix preauth hash corruption fs/cifs/cifs_swn.c | 2 +- fs/cifs/cifsacl.c| 9 ++--- fs/cifs/fs_context.c | 6 -- fs/cifs/inode.c | 10 +- fs/cifs/transport.c | 7 ++- 5 files changed, 26 insertions(+), 8 deletions(-) -- Thanks, Steve
Re: [PATCH] fs/cifs/: fix misspellings using codespell tool
merged into cifs-2.6.git for-next On Thu, Mar 18, 2021 at 7:50 PM wrote: > > From: Liu xuzhi > > A typo is found out by codespell tool in 251th lines of cifs_swn.c: > > $ codespell ./fs/cifs/ > ./cifs_swn.c:251: funciton ==> function > > Fix a typo found by codespell. > > Signed-off-by: Liu xuzhi > --- > fs/cifs/cifs_swn.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/cifs_swn.c b/fs/cifs/cifs_swn.c > index f2d730fffccb..d829b8bf833e 100644 > --- a/fs/cifs/cifs_swn.c > +++ b/fs/cifs/cifs_swn.c > @@ -248,7 +248,7 @@ static int cifs_swn_send_unregister_message(struct > cifs_swn_reg *swnreg) > > /* > * Try to find a matching registration for the tcon's server name and share > name. > - * Calls to this funciton must be protected by cifs_swnreg_idr_mutex. > + * Calls to this function must be protected by cifs_swnreg_idr_mutex. > * TODO Try to avoid memory allocations > */ > static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon) > -- > 2.25.1 > -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commits in the cifsd tree
Namjae updated/restructured the initial list of patches to be easier to follow (and also fixed the missing Acked-by and Signed-off-by tags) - and I have updated the branch. Let us know if you see other cleanup that would make this (initial merge) process easier. I am continuing to run testing on it, but it is looking promising. On Tue, Mar 16, 2021 at 5:37 PM Stephen Rothwell wrote: > > Hi all, > > Commits > > 5c6ab7c9c301 ("cifsd: don't clear ATTR_DIRECTORY with ATTR_NORMAL + other > bits") > 37f12f1a9e76 ("cifsd: remove backup file lookup restriction") > 511fbebc9d3b ("cifsd: fix racy issue between close_id_del_oplock() and > destroy_lease_table()") > 60f4e4de7868 ("cifsd: handle idmapped mounts") > 8e1b809f11ee ("cifsd: use notify_change in set_file_basic_info()") > 8c1930014c34 ("cifsd: fix the issue of change the directory to the file") > 608b7bb0e414 ("cifsd: fix potential use after free in > ksmbd_vfs_set_init_posix_acl()") > a53327ff87b2 ("cifsd: fix memleak in ksmbd_vfs_set_init_posix_acl()") > a98dbc20cbdb ("cifsd: Add missing path_put() calls in smb2_open()") > ca46f5254981 ("cifsd: Fix error handling in __ksmbd_vfs_rename") > bb932cbbc781 ("cifsd: Do not print timestamp after processing Maximum > Access CreateContext") > 76cd471c09e7 ("cifsd: add v4 dos attribute structure") > f2b993deb1cd ("cifsd: fix random failure from smb2.create.multi") > 01a8bf060081 ("cifsd: fix smb2.lease.statopen3 in smbtorture") > 534cf891fdd8 ("cifsd: Remove call to ksmbd_revert_fsids() in > ksmbd_vfs_mkdir()") > 3a1bb9e2ccc3 ("cifsd: Make sure ksmbd_override_fsids() is called with > Durable Opens") > d69623ac1609 ("cifsd: Add missing dput() in process_query_dir_entries()") > 7caaa1dc0f84 ("cifsd: Fix incorrect error handling in smb2_open()") > 6b457e969def ("cifsd: handle an error case in extract_last_component") > 57987aee520c ("cifsd: fix a typo error in the name of a funnction") > 56f3ac5868fa ("cifsd: fix SMB2_QUERY_MAXIMAL_ACCESS_REQUEST") > e36e95b4a918 ("cifsd: implement maximal allowed desired access") > 1b9f1e136e8b ("cifsd: fix build break with linux-5.11 kernel") > 4a26a8aa4189 ("cifsd: macros with complex values should be enclosed in > parentheses") > affbd69c2cb5 ("cifsd: make xattr format of ksmbd compatible with samba's > one") > b0467288c502 ("cifsd: fix a memleak from netdevice_notifier") > 1e5eb460e3e7 ("cifsd: Use netdevice_notifier to configure TCP listeners") > 58dcf3a3aac6 ("cifsd: Change alloc_iface() return type in transport_tcp.c") > eaa4fa7ab91d ("cifsd: avoid calling ksmbd_override_fsids recursively") > 8d1b5f668026 ("cifsd: set supplementary groups when overriding credentials") > 901ce4507f08 ("cifsd: fix integer overflow issue in ksmbd_vfs_fqar_lseek()") > d76abe30e48d ("cifsd: downgrade "unexpected oplock state" to a debug > message") > f6881be8859a ("cifsd: fix racy access to ksmbd_file") > 5441ad0cfa45 ("cifsd: prevent ksmbd from sending duplicate oplock break > notifications") > 62a5df744681 ("cifsd: enable SMB_SERVER_CHECK_CAP_NET_ADMIN by default") > 597357deeecf ("cifsd: update cifsd.rst file") > 9fe8dd069399 ("cifsd: set SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB flags in smb2 > close response if attribute fields are valid") > fdcdac101a2d ("cifsd: fill times in SMB2_CLOSE response") > 1e831f7ccab5 ("cifsd: fix permission denied error when querying dacl") > ec03266638d3 ("cifsd: rename FILE_{READ/WRITE}_DESIRED_ACCESS") > 96f1ad1dccf8 ("cifsd: add support for recognizing WSL reparse tags") > e7ffe8d67f4b ("cifsd: set O_PATH to open_flags if desired access is > FILE_READ_ATTRIBUTES") > c98dcf7abaf6 ("cifsd: fix unlink permission error") > 4696fa2903b6 ("cifsd: fix stat permission error") > ba2513ea3367 ("cifsd: fix invalid open flags") > 3ed1e35ea23a ("cifsd: wrap vfs acls functions with CONFIG_FS_POSIX_ACL") > 0460c98d086e ("cifsd: open file with O_NONBLOCK flags by default") > 8aecd712e0a4 ("cifsd: remove repeated word") > 0db918db34a4 ("cifsd: replace ENOTSUPP with EOPNOTSUPP") > ace29fa5db87 ("cifsd: add missing create posix context in response") > 3cbb1a2dc880 ("cifsd: add smb2 POSIX query dir") > 6139ab2e0432 ("cifsd: add support for statfs for smb3.1.1 posix extensions") > f49a9cd0596e ("cifsd: add support for query info using posix extensions > (level 100)") > e19a174afef6 ("cifsd: add SPNEGO-based Kerberos 5 authentication") > affb750272ff ("cifsd: add IPC command for authentication") > 55f9e825b44b ("cifsd: select the preferred authentication mechanism among > proposal") > ef24dca82789 ("cifsd: add support for ACLs") > 5c240146de09 ("cifsd: get subauth values generated by ksmbd.mountd") > 14246142f631 ("cifsd: add support for lsarpc rpc") > 5b0cebbceca5 ("cifsd: add support for samr rpc") > 11a01d92f833 ("cifsd: fix racy issue between kill server command and > destroy_previous_session()") > 504e2b3e74e8 ("cifsd: fix wrong goto statement
[GIT PULL] cifs fixes
Please pull the following changes since commit a38fd8748464831584a19438cbb3082b5a2dab15: Linux 5.12-rc2 (2021-03-05 17:33:41 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-rc2-smb3 for you to fetch changes up to 04ad69c342fc4de5bd23be9ef15ea7574fb1a87e: cifs: do not send close in compound create+close requests (2021-03-08 21:23:22 -0600) 6 cifs/smb3 fixes, 3 for stable, including some important mulitchannel crediting fixes, and fix for statfs error handling Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/524 Aurelien Aptel (2): cifs: fix credit accounting for extra channel cifs: ask for more credit on async read/write code paths Paulo Alcantara (4): cifs: print MIDs in decimal notation cifs: change noisy error message to FYI cifs: return proper error code in statfs(2) cifs: do not send close in compound create+close requests fs/cifs/cifs_debug.c | 2 +- fs/cifs/cifsfs.c | 2 +- fs/cifs/cifsglob.h | 19 ++- fs/cifs/connect.c| 14 +++--- fs/cifs/sess.c | 1 + fs/cifs/smb2inode.c | 1 + fs/cifs/smb2misc.c | 10 +- fs/cifs/smb2ops.c| 10 +- fs/cifs/smb2pdu.c| 6 ++ fs/cifs/smb2proto.h | 3 +-- fs/cifs/transport.c | 4 ++-- 11 files changed, 36 insertions(+), 36 deletions(-) -- Thanks, Steve
Re: [PATCH] copy_file_range.2: Kernel v5.12 updates
On Sun, Feb 28, 2021 at 1:36 AM Amir Goldstein wrote: > > On Sun, Feb 28, 2021 at 1:08 AM Steve French wrote: > > > > On Fri, Feb 26, 2021 at 11:43 PM Amir Goldstein wrote: > > > > > > On Sat, Feb 27, 2021 at 12:19 AM Alejandro Colomar (man-pages) > > > wrote: > > > > > > > > Hello Amir, Luis, > > > > > > > > On 2/24/21 5:10 PM, Amir Goldstein wrote: > > > > > On Wed, Feb 24, 2021 at 4:22 PM Luis Henriques > > > > > wrote: > > > > >> > > > > >> Update man-page with recent changes to this syscall. > > > > >> > > > > >> Signed-off-by: Luis Henriques > > > > >> --- > > > > >> Hi! > > > > >> > > > > >> Here's a suggestion for fixing the manpage for copy_file_range(). > > > > >> Note that > > > > >> I've assumed the fix will hit 5.12. > > > > >> > > > > >> man2/copy_file_range.2 | 10 +- > > > > >> 1 file changed, 9 insertions(+), 1 deletion(-) > > > > >> > > > > >> diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2 > > > > >> index 611a39b8026b..b0fd85e2631e 100644 > > > > >> --- a/man2/copy_file_range.2 > > > > >> +++ b/man2/copy_file_range.2 > > > > >> @@ -169,6 +169,9 @@ Out of memory. > > > > >> .B ENOSPC > > > > >> There is not enough space on the target filesystem to complete the > > > > >> copy. > > > > >> .TP > > > > >> +.B EOPNOTSUPP > > > > > > > > I'll add the kernel version here: > > > > > > > > .BR EOPNOTSUPP " (since Linux 5.12)" > > > > > > Error could be returned prior to 5.3 and would be probably returned > > > by future stable kernels 5.3..5.12 too > > > > > > > > > > > >> +The filesystem does not support this operation >> +.TP > > > > >> .B EOVERFLOW > > > > >> The requested source or destination range is too large to > > > > >> represent in the > > > > >> specified data types. > > > > >> @@ -187,7 +190,7 @@ refers to an active swap file. > > > > >> .B EXDEV > > > > >> The files referred to by > > > > >> .IR fd_in " and " fd_out > > > > >> -are not on the same mounted filesystem (pre Linux 5.3). > > > > >> +are not on the same mounted filesystem (pre Linux 5.3 and post > > > > >> Linux 5.12). > > > > > > > > I'm not sure that 'mounted' adds any value here. Would you remove the > > > > word here? > > > > > > See rename(2). 'mounted' in this context is explained there. > > > HOWEVER, it does not fit here. > > > copy_file_range() IS allowed between two mounts of the same filesystem > > > instance. > > > > > > To make things more complicated, it appears that cross mount clone is not > > > allowed via FICLONE/FICLONERANGE ioctl, so ioctl_ficlonerange(2) man page > > > also uses the 'mounted filesystem' terminology for EXDEV > > > > > > As things stand now, because of the fallback to clone logic, > > > copy_file_range() provides a way for users to clone across different > > > mounts > > > of the same filesystem instance, which they cannot do with the FICLONE > > > ioctl. > > > > > > Fun :) > > > > > > BTW, I don't know if preventing cross mount clone was done intentionally, > > > but as I wrote in a comment in the code once: > > > > > > /* > > > * FICLONE/FICLONERANGE ioctls enforce that src and dest files > > > are on > > > * the same mount. Practically, they only need to be on the same > > > file > > > * system. > > > */ > > > > > > > > > > > It reads as if two separate devices with the same filesystem type would > > > > still give this error. > > > > > > > > Per the LWN.net article Amir shared, this is permitted ("When called > > > > from user space, copy_file_range() will only try to copy a file across > > > > filesystems if the two are of the same type"). > > > > > > > > This behavior was slightly different be
Re: [PATCH] copy_file_range.2: Kernel v5.12 updates
On Fri, Feb 26, 2021 at 11:43 PM Amir Goldstein wrote: > > On Sat, Feb 27, 2021 at 12:19 AM Alejandro Colomar (man-pages) > wrote: > > > > Hello Amir, Luis, > > > > On 2/24/21 5:10 PM, Amir Goldstein wrote: > > > On Wed, Feb 24, 2021 at 4:22 PM Luis Henriques wrote: > > >> > > >> Update man-page with recent changes to this syscall. > > >> > > >> Signed-off-by: Luis Henriques > > >> --- > > >> Hi! > > >> > > >> Here's a suggestion for fixing the manpage for copy_file_range(). Note > > >> that > > >> I've assumed the fix will hit 5.12. > > >> > > >> man2/copy_file_range.2 | 10 +- > > >> 1 file changed, 9 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2 > > >> index 611a39b8026b..b0fd85e2631e 100644 > > >> --- a/man2/copy_file_range.2 > > >> +++ b/man2/copy_file_range.2 > > >> @@ -169,6 +169,9 @@ Out of memory. > > >> .B ENOSPC > > >> There is not enough space on the target filesystem to complete the > > >> copy. > > >> .TP > > >> +.B EOPNOTSUPP > > > > I'll add the kernel version here: > > > > .BR EOPNOTSUPP " (since Linux 5.12)" > > Error could be returned prior to 5.3 and would be probably returned > by future stable kernels 5.3..5.12 too > > > > > >> +The filesystem does not support this operation >> +.TP > > >> .B EOVERFLOW > > >> The requested source or destination range is too large to represent in > > >> the > > >> specified data types. > > >> @@ -187,7 +190,7 @@ refers to an active swap file. > > >> .B EXDEV > > >> The files referred to by > > >> .IR fd_in " and " fd_out > > >> -are not on the same mounted filesystem (pre Linux 5.3). > > >> +are not on the same mounted filesystem (pre Linux 5.3 and post Linux > > >> 5.12). > > > > I'm not sure that 'mounted' adds any value here. Would you remove the > > word here? > > See rename(2). 'mounted' in this context is explained there. > HOWEVER, it does not fit here. > copy_file_range() IS allowed between two mounts of the same filesystem > instance. > > To make things more complicated, it appears that cross mount clone is not > allowed via FICLONE/FICLONERANGE ioctl, so ioctl_ficlonerange(2) man page > also uses the 'mounted filesystem' terminology for EXDEV > > As things stand now, because of the fallback to clone logic, > copy_file_range() provides a way for users to clone across different mounts > of the same filesystem instance, which they cannot do with the FICLONE ioctl. > > Fun :) > > BTW, I don't know if preventing cross mount clone was done intentionally, > but as I wrote in a comment in the code once: > > /* > * FICLONE/FICLONERANGE ioctls enforce that src and dest files are on > * the same mount. Practically, they only need to be on the same file > * system. > */ > > > > > It reads as if two separate devices with the same filesystem type would > > still give this error. > > > > Per the LWN.net article Amir shared, this is permitted ("When called > > from user space, copy_file_range() will only try to copy a file across > > filesystems if the two are of the same type"). > > > > This behavior was slightly different before 5.3 AFAICR (was it?) ("until > > then, copy_file_range() refused to copy between files that were not > > located on the same filesystem."). If that's the case, I'd specify the > > difference, or more probably split the error into two, one before 5.3, > > and one since 5.12. > > > > True. > > > > > > > I think you need to drop the (Linux range) altogether. > > > > I'll keep the range. Users of 5.3..5.11 might be surprised if the > > filesystems are different and they don't get an error, I think. > > > > I reworded it to follow other pages conventions: > > > > .BR EXDEV " (before Linux 5.3; or since Linux 5.12)" > > > > which renders as: > > > > EXDEV (before Linux 5.3; or since Linux 5.12) > >The files referred to by fd_in and fd_out are not on > >the same mounted filesystem. > > > > drop 'mounted' > > > > > > What's missing here is the NFS cross server copy use case. > > > Maybe: At least for the SMB3 kernel server (ksmbd "cifsd") looks like they use splice. And for the user space CIFS/SMB3 server (like Samba) they have a configurable plug in library interface ("Samba VFS modules") that would allow you to implement cross filesystem copy optimally for your version of Linux and plug this into Samba with little work on your part. > > > > Again, this wasn't true before 5.3, right? > > > > Right. > Actually, v5.3 provides the vfs capabilities for filesystems to support > cross fs copy. I am not sure if NFS already implements cross fs copy in > v5.3 and not sure about cifs. Need to get input from nfs/cis developers > or dig in the release notes for server-side copy. The SMB3 protocol has multiple ways to do "server side copy" (copy offload to the server), some of which would apply to your example. The case of "reflink" in many cases would be most
Current mainline
Has anyone seen this? I was running some xfstests today on current mainline (ran same tests yesterday and multiple times last week on 5.11 and did not see this) and hit this multiple times [ 287.702292] [ cut here ] [ 287.702296] WARNING: CPU: 5 PID: 8223 at drivers/gpu/drm/ttm/ttm_bo.c:512 ttm_bo_release+0x2f7/0x350 [ttm] [ 287.702309] Modules linked in: cifs ccm md4 cmac nls_utf8 libarc4 libdes rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat iptable_mangle iptable_raw iptable_security nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nfnetlink ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter sunrpc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel virtio_balloon ip_tables xfs qxl drm_ttm_helper ttm drm_kms_helper cec drm virtio_net net_failover floppy failover virtio_blk virtio_console crc32c_intel qemu_fw_cfg [last unloaded: cifs] [ 287.702352] CPU: 5 PID: 8223 Comm: kworker/5:50 Tainted: GW 5.11.0 #1 [ 287.702355] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 [ 287.702357] Workqueue: events qxl_gc_work [qxl] [ 287.702362] RIP: 0010:ttm_bo_release+0x2f7/0x350 [ttm] [ 287.702367] Code: e9 6a fd ff ff e8 c9 e7 46 eb e9 92 fd ff ff 49 8b 7f 90 b9 30 75 00 00 31 d2 be 01 00 00 00 e8 1f 0b 47 eb 49 8b 47 e0 eb a1 <0f> 0b 49 8d 77 08 41 c7 87 94 00 00 00 00 00 00 00 31 d2 4c 89 f7 [ 287.702370] RSP: 0018:b2c2c877fda8 EFLAGS: 00010202 [ 287.702372] RAX: 0001 RBX: 000c RCX: 000d [ 287.702373] RDX: 0001 RSI: 0246 RDI: c035a0e8 [ 287.702374] RBP: 922102e4c688 R08: 92210f996050 R09: 922110ac5938 [ 287.702376] R10: R11: 922100469240 R12: 922110ac5e80 [ 287.702377] R13: 92210b5509d0 R14: 92210b550800 R15: 92210b550968 [ 287.702383] FS: () GS:92252bd4() knlGS: [ 287.702385] CS: 0010 DS: ES: CR0: 80050033 [ 287.702386] CR2: 55617142e5d0 CR3: 000334610005 CR4: 003706e0 [ 287.702400] DR0: DR1: DR2: [ 287.702401] DR3: DR6: fffe0ff0 DR7: 0400 [ 287.702403] Call Trace: [ 287.702408] qxl_bo_unref+0x3a/0x50 [qxl] [ 287.702414] qxl_release_free_list+0x49/0x90 [qxl] [ 287.702419] qxl_release_free+0x72/0xe0 [qxl] [ 287.702423] qxl_garbage_collect+0xd1/0x180 [qxl] [ 287.702427] process_one_work+0x1c0/0x390 [ 287.702438] worker_thread+0x30/0x390 [ 287.702441] ? process_one_work+0x390/0x390 [ 287.702444] kthread+0x117/0x130 [ 287.702448] ? kthread_park+0x90/0x90 [ 287.702450] ret_from_fork+0x22/0x30 [ 287.702457] ---[ end trace 03e19399d00180a1 ]--- [ 287.702539] [ cut here ] -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit f40ddce88593482919761f74910f42f4b84c004b: Linux 5.11 (2021-02-14 14:32:24 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.12-smb3-part1 for you to fetch changes up to 8369dfd7841e70711c53a065ffb8029f24520200: cifs: update internal version number (2021-02-25 19:08:11 -0600) cifs/smb3 fixes: - improvements to mode bit conversion, chmod and chown when using cifsacl mount option - two new mount options for controlling attribute caching - improvements to crediting and reconnect, improved debugging - reconnect fix - add SMB3.1.1 dialect to default dialects for vers=3 Still working on a security fix and some multichannel fixes which are not included in this pull request Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/513 David Howells (1): cifs: use discard iterator to discard unneeded network data more efficiently Paulo Alcantara (4): cifs: fix nodfs mount option cifs: fix DFS failover cifs: check all path components in resolved dfs target cifs: introduce helper for finding referral server to improve DFS target resolution Rohith Surabattula (1): TCON Reconnect during STATUS_NETWORK_NAME_DELETED Ronnie Sahlberg (1): cifs: fix handling of escaped ',' in the password mount argument Shyam Prasad N (8): cifs: New optype for session operations. cifs: Fix in error types returned for out-of-credit situations. cifs: Identify a connection by a conn_id. cifs: Reformat DebugData and index connections by conn_id. cifs: Fix cifsacl ACE mask for group and others. cifs: Retain old ACEs when converting between mode bits and ACL. cifs: Change SIDs in ACEs while transferring file ownership. cifs: If a corrupted DACL is returned by the server, bail out. Steve French (11): smb3: negotiate current dialect (SMB3.1.1) when version 3 or greater requested cifs: fix trivial typo cifs: documentation cleanup cifs: change confusing field serverName (to ip_addr) cifs: clarify hostname vs ip address in /proc/fs/cifs/DebugData cifs: cleanup a few le16 vs. le32 uses in cifsacl.c cifs: minor simplification to smb2_is_network_name_deleted cifs: Add new mount parameter "acdirmax" to allow caching directory metadata cifs: convert revalidate of directories to using directory metadata cache timeout cifs: Add new parameter "acregmax" for distinct file and directory metadata timeout cifs: update internal version number YueHaibing (1): cifs: Fix inconsistent IS_ERR and PTR_ERR Documentation/admin-guide/cifs/authors.rst | 6 +- Documentation/admin-guide/cifs/changes.rst | 5 +- Documentation/admin-guide/cifs/introduction.rst | 30 +- Documentation/admin-guide/cifs/todo.rst | 34 ++- Documentation/admin-guide/cifs/usage.rst| 2 +- fs/cifs/cifs_debug.c| 121 fs/cifs/cifs_swn.c | 2 +- fs/cifs/cifsacl.c | 379 +++- fs/cifs/cifsacl.h | 4 +- fs/cifs/cifsencrypt.c | 6 +- fs/cifs/cifsfs.c| 15 +- fs/cifs/cifsfs.h| 2 +- fs/cifs/cifsglob.h | 11 +- fs/cifs/cifsproto.h | 2 + fs/cifs/cifssmb.c | 6 +- fs/cifs/connect.c | 301 ++- fs/cifs/dfs_cache.c | 33 ++- fs/cifs/file.c | 2 +- fs/cifs/fs_context.c| 75 +++-- fs/cifs/fs_context.h| 6 +- fs/cifs/inode.c | 23 +- fs/cifs/sess.c | 2 +- fs/cifs/smb2ops.c | 109 +-- fs/cifs/smb2pdu.c | 22 +- fs/cifs/trace.h | 36 ++- fs/cifs/transport.c | 63 ++-- 26 files changed, 883 insertions(+), 414 deletions(-) -- Thanks, Steve
Re: [EXTERNAL] re: cifs: Retain old ACEs when converting between mode bits and ACL.
Add the RB from Rohith and merged into cifs-2.6.git for-next On Wed, Feb 24, 2021 at 10:58 AM Shyam Prasad N via samba-technical wrote: > > Hi Steve, > > Please accept this fix for the bug that Colin pointed out. > This can be hit if the server has a corrupted SD, or it got corrupted > over the network. > We used to ignore the ACL in such a case (which in combination with my > patches caused the issue). But I think we should be returning an error > immediately. > > Regards, > Shyam > > On Wed, Feb 24, 2021 at 7:16 AM Shyam Prasad > wrote: > > > > Hi Colin, > > > > Thanks for reporting this. I'll submit a fix. > > > > Regards, > > Shyam > > > > -Original Message- > > From: Colin Ian King > > Sent: Wednesday, February 24, 2021 6:14 PM > > To: Shyam Prasad > > Cc: Steve French ; linux-c...@vger.kernel.org; > > samba-techni...@lists.samba.org; linux-kernel@vger.kernel.org > > Subject: [EXTERNAL] re: cifs: Retain old ACEs when converting between mode > > bits and ACL. > > > > Hi, > > > > Static analysis on linux-next with Coverity had detected a potential null > > pointer dereference with the following commit: > > > > commit f5065508897a922327f32223082325d10b069ebc > > Author: Shyam Prasad N > > Date: Fri Feb 12 04:38:43 2021 -0800 > > > > cifs: Retain old ACEs when converting between mode bits and ACL. > > > > The analysis is as follows: > > > > 1258 /* Convert permission bits from mode to equivalent CIFS ACL */ > > 1259 static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd > > *pnntsd, > > 1260__u32 secdesclen, __u32 *pnsecdesclen, __u64 *pnmode, kuid_t > > uid, kgid_t gid, > > 1261bool mode_from_sid, bool id_from_sid, int *aclflag) > > 1262 { > > 1263int rc = 0; > > 1264__u32 dacloffset; > > 1265__u32 ndacloffset; > > 1266__u32 sidsoffset; > > 1267struct cifs_sid *owner_sid_ptr, *group_sid_ptr; > > 1268struct cifs_sid *nowner_sid_ptr = NULL, *ngroup_sid_ptr = NULL; > > > > 1. assign_zero: Assigning: dacl_ptr = NULL. > > > > 1269struct cifs_acl *dacl_ptr = NULL; /* no need for SACL ptr */ > > 1270struct cifs_acl *ndacl_ptr = NULL; /* no need for SACL ptr */ > > 1271char *end_of_acl = ((char *)pntsd) + secdesclen; > > 1272u16 size = 0; > > 1273 > > 1274dacloffset = le32_to_cpu(pntsd->dacloffset); > > > > 2. Condition dacloffset, taking false branch. > > > > 1275if (dacloffset) { > > 1276dacl_ptr = (struct cifs_acl *)((char *)pntsd + > > dacloffset); > > 1277if (end_of_acl < (char *)dacl_ptr + > > le16_to_cpu(dacl_ptr->size)) { > > 1278cifs_dbg(VFS, "Existing ACL size is wrong. > > Discarding old ACL\n"); > > 1279dacl_ptr = NULL; > > > > NOTE: dacl_ptr is set to NULL and dacloffset is true > > > > 1280} > > 1281} > > 1282 > > 1283owner_sid_ptr = (struct cifs_sid *)((char *)pntsd + > > 1284le32_to_cpu(pntsd->osidoffset)); > > 1285group_sid_ptr = (struct cifs_sid *)((char *)pntsd + > > 1286le32_to_cpu(pntsd->gsidoffset)); > > 1287 > > > > 3. Condition pnmode, taking true branch. > > 4. Condition *pnmode != 18446744073709551615ULL, taking false branch. > > > > 1288if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ > > 1289ndacloffset = sizeof(struct cifs_ntsd); > > 1290ndacl_ptr = (struct cifs_acl *)((char *)pnntsd + > > ndacloffset); > > 1291ndacl_ptr->revision = > > 1292dacloffset ? dacl_ptr->revision : > > cpu_to_le16(ACL_REVISION); > > 1293 > > 1294ndacl_ptr->size = cpu_to_le16(0); > > 1295ndacl_ptr->num_aces = cpu_to_le32(0); > > 1296 > > 1297rc = set_chmod_dacl(dacl_ptr, ndacl_ptr, > > owner_sid_ptr, group_sid_ptr, > > 1298pnmode, mode_from_sid); > > 1299 > > 1300sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size); > > 1301/* copy the non-dacl portion of secdesc */ > > 1302*pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset, > > 1303NULL, NULL); > > 1304 &g
Re: [PATCH 00/33] Network fs helper library & fscache kiocb API [ver #3]
On Tue, Feb 23, 2021 at 2:28 PM Matthew Wilcox wrote: > > On Mon, Feb 15, 2021 at 11:22:20PM -0600, Steve French wrote: > > On Mon, Feb 15, 2021 at 8:10 PM Matthew Wilcox wrote: > > > The switch from readpages to readahead does help in a couple of corner > > > cases. For example, if you have two processes reading the same file at > > > the same time, one will now block on the other (due to the page lock) > > > rather than submitting a mess of overlapping and partial reads. > > > > Do you have a simple repro example of this we could try (fio, dbench, iozone > > etc) to get some objective perf data? > > I don't. The problem was noted by the f2fs people, so maybe they have a > reproducer. > > > My biggest worry is making sure that the switch to netfs doesn't degrade > > performance (which might be a low bar now since current network file copy > > perf seems to signifcantly lag at least Windows), and in some easy to > > understand > > scenarios want to make sure it actually helps perf. > > I had a question about that ... you've mentioned having 4x4MB reads > outstanding as being the way to get optimum performance. Is there a > significant performance difference between 4x4MB, 16x1MB and 64x256kB? > I'm concerned about having "too large" an I/O on the wire at a given time. > For example, with a 1Gbps link, you get 250MB/s. That's a minimum > latency of 16us for a 4kB page, but 16ms for a 4MB page. > > "For very simple tasks, people can perceive latencies down to 2 ms or less" > (https://danluu.com/input-lag/) > so going all the way to 4MB I/Os takes us into the perceptible latency > range, whereas a 256kB I/O is only 1ms. > > So could you do some experiments with fio doing direct I/O to see if > it takes significantly longer to do, say, 1TB of I/O in 4MB chunks vs > 256kB chunks? Obviously use threads to keep lots of I/Os outstanding. That is a good question and it has been months since I have done experiments with something similar. Obviously this will vary depending on RDMA or not and multichannel or not - but assuming the 'normal' low end network configuration - ie a 1Gbps link and no RDMA or multichannel I could do some more recent experiments. In the past what I had noticed was that server performance for simple workloads like cp or grep increased with network I/O size to a point: smaller than 256K packet size was bad. Performance improved significantly from 256K to 512K to 1MB, but only very slightly from 1MB to 2MB to 4MB and sometimes degraded at 8MB (IIRC 8MB is the max commonly supported by SMB3 servers), but this is with only one adapter (no multichannel) and 1Gb adapters. But in those examples there wasn't a lot of concurrency on the wire. I did some experiments with increasing the read ahead size (which causes more than one async read to be issued by cifs.ko but presumably does still result in some 'dead time') which seemed to help perf of some sequential read examples (e.g. grep or cp) to some servers but I didn't try enough variety of server targets to feel confident about that change especially if netfs is coming e.g. a change I experimented with was: sb->s_bdi->ra_pages = cifs_sb->ctx->rsize / PAGE_SIZE to sb->s_bdi->ra_pages = 2 * cifs_sb->ctx->rsize / PAGE_SIZE and it did seem to help a little. I would expect that 8x1MB (ie trying to keep eight 1MB reads in process should keep the network mostly busy and not lead to too much dead time on server, client or network) and is 'good enough' in many read ahead use cases (at least for non-RDMA, and non-multichannel on a slower network) to keep the pipe file, and I would expect the performance to be similar to the equivalent using 2MB read (e.g. 4x2MB) and perhaps better than 2x4MB. Below 1MB i/o size on the wire I would expect to see degradation due to packet processing and task switching overhead. Would definitely be worth doing more experimentation here. -- Thanks, Steve
Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices
On Thu, Feb 18, 2021 at 4:03 AM Amir Goldstein wrote: > > On Thu, Feb 18, 2021 at 9:42 AM Christoph Hellwig wrote: > > > > Looks good: > > > > Reviewed-by: Christoph Hellwig > > > > This whole idea of cross-device copie has always been a horrible idea, > > and I've been arguing against it since the patches were posted. > > Ok. I'm good with this v2 as well, but need to add the fallback to > do_splice_direct() > in nfsd_copy_file_range(), because this patch breaks it. Interestingly, for ksmbd (cifsd) looks like they already do splice not copy_file_range -- Thanks, Steve
Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices
On Tue, Feb 16, 2021 at 1:40 PM Amir Goldstein wrote: > > On Tue, Feb 16, 2021 at 9:31 PM Steve French wrote: > > > > On Tue, Feb 16, 2021 at 1:29 PM Anna Schumaker > > wrote: > > > > > > On Tue, Feb 16, 2021 at 2:22 PM Amir Goldstein wrote: > > > > > > > > On Tue, Feb 16, 2021 at 8:54 PM Luis Henriques > > > > wrote: > > > > > > > > > > Amir Goldstein writes: > > > > > > > > > > > On Tue, Feb 16, 2021 at 6:41 PM Luis Henriques > > > > > > wrote: > > > > > >> > > > > > >> Amir Goldstein writes: > > > > > >> > > > > > >> >> Ugh. And I guess overlayfs may have a similar problem. > > > > > >> > > > > > > >> > Not exactly. > > > > > >> > Generally speaking, overlayfs should call vfs_copy_file_range() > > > > > >> > with the flags it got from layer above, so if called from nfsd it > > > > > >> > will allow cross fs copy and when called from syscall it won't. > > > > > >> > > > > > > >> > There are some corner cases where overlayfs could benefit from > > > > > >> > COPY_FILE_SPLICE (e.g. copy from lower file to upper file), but > > > > > >> > let's leave those for now. Just leave overlayfs code as is. > > > > > >> > > > > > >> Got it, thanks for clarifying. > > > > > >> > > > > > >> >> > This is easy to solve with a flag COPY_FILE_SPLICE (or > > > > > >> >> > something) that > > > > > >> >> > is internal to kernel users. > > > > > >> >> > > > > > > >> >> > FWIW, you may want to look at the loop in ovl_copy_up_data() > > > > > >> >> > for improvements to nfsd_copy_file_range(). > > > > > >> >> > > > > > > >> >> > We can move the check out to copy_file_range syscall: > > > > > >> >> > > > > > > >> >> > if (flags != 0) > > > > > >> >> > return -EINVAL; > > > > > >> >> > > > > > > >> >> > Leave the fallback from all filesystems and check for the > > > > > >> >> > COPY_FILE_SPLICE flag inside generic_copy_file_range(). > > > > > >> >> > > > > > >> >> Ok, the diff bellow is just to make sure I understood your > > > > > >> >> suggestion. > > > > > >> >> > > > > > >> >> The patch will also need to: > > > > > >> >> > > > > > >> >> - change nfs and overlayfs calls to vfs_copy_file_range() so > > > > > >> >> that they > > > > > >> >>use the new flag. > > > > > >> >> > > > > > >> >> - check flags in generic_copy_file_checks() to make sure only > > > > > >> >> valid flags > > > > > >> >>are used (COPY_FILE_SPLICE at the moment). > > > > > >> >> > > > > > >> >> Also, where should this flag be defined? > > > > > >> >> include/uapi/linux/fs.h? > > > > > >> > > > > > > >> > Grep for REMAP_FILE_ > > > > > >> > Same header file, same Documentation rst file. > > > > > >> > > > > > > >> >> > > > > > >> >> Cheers, > > > > > >> >> -- > > > > > >> >> Luis > > > > > >> >> > > > > > >> >> diff --git a/fs/read_write.c b/fs/read_write.c > > > > > >> >> index 75f764b43418..341d315d2a96 100644 > > > > > >> >> --- a/fs/read_write.c > > > > > >> >> +++ b/fs/read_write.c > > > > > >> >> @@ -1383,6 +1383,13 @@ ssize_t generic_copy_file_range(struct > > > > > >> >> file *file_in, loff_t pos_in, > > > > > >> >> struct file *file_out, loff_t > > > > > >> >> pos_out, >
Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices
On Tue, Feb 16, 2021 at 1:29 PM Anna Schumaker wrote: > > On Tue, Feb 16, 2021 at 2:22 PM Amir Goldstein wrote: > > > > On Tue, Feb 16, 2021 at 8:54 PM Luis Henriques wrote: > > > > > > Amir Goldstein writes: > > > > > > > On Tue, Feb 16, 2021 at 6:41 PM Luis Henriques > > > > wrote: > > > >> > > > >> Amir Goldstein writes: > > > >> > > > >> >> Ugh. And I guess overlayfs may have a similar problem. > > > >> > > > > >> > Not exactly. > > > >> > Generally speaking, overlayfs should call vfs_copy_file_range() > > > >> > with the flags it got from layer above, so if called from nfsd it > > > >> > will allow cross fs copy and when called from syscall it won't. > > > >> > > > > >> > There are some corner cases where overlayfs could benefit from > > > >> > COPY_FILE_SPLICE (e.g. copy from lower file to upper file), but > > > >> > let's leave those for now. Just leave overlayfs code as is. > > > >> > > > >> Got it, thanks for clarifying. > > > >> > > > >> >> > This is easy to solve with a flag COPY_FILE_SPLICE (or something) > > > >> >> > that > > > >> >> > is internal to kernel users. > > > >> >> > > > > >> >> > FWIW, you may want to look at the loop in ovl_copy_up_data() > > > >> >> > for improvements to nfsd_copy_file_range(). > > > >> >> > > > > >> >> > We can move the check out to copy_file_range syscall: > > > >> >> > > > > >> >> > if (flags != 0) > > > >> >> > return -EINVAL; > > > >> >> > > > > >> >> > Leave the fallback from all filesystems and check for the > > > >> >> > COPY_FILE_SPLICE flag inside generic_copy_file_range(). > > > >> >> > > > >> >> Ok, the diff bellow is just to make sure I understood your > > > >> >> suggestion. > > > >> >> > > > >> >> The patch will also need to: > > > >> >> > > > >> >> - change nfs and overlayfs calls to vfs_copy_file_range() so that > > > >> >> they > > > >> >>use the new flag. > > > >> >> > > > >> >> - check flags in generic_copy_file_checks() to make sure only > > > >> >> valid flags > > > >> >>are used (COPY_FILE_SPLICE at the moment). > > > >> >> > > > >> >> Also, where should this flag be defined? include/uapi/linux/fs.h? > > > >> > > > > >> > Grep for REMAP_FILE_ > > > >> > Same header file, same Documentation rst file. > > > >> > > > > >> >> > > > >> >> Cheers, > > > >> >> -- > > > >> >> Luis > > > >> >> > > > >> >> diff --git a/fs/read_write.c b/fs/read_write.c > > > >> >> index 75f764b43418..341d315d2a96 100644 > > > >> >> --- a/fs/read_write.c > > > >> >> +++ b/fs/read_write.c > > > >> >> @@ -1383,6 +1383,13 @@ ssize_t generic_copy_file_range(struct file > > > >> >> *file_in, loff_t pos_in, > > > >> >> struct file *file_out, loff_t > > > >> >> pos_out, > > > >> >> size_t len, unsigned int flags) > > > >> >> { > > > >> >> + if (!(flags & COPY_FILE_SPLICE)) { > > > >> >> + if (!file_out->f_op->copy_file_range) > > > >> >> + return -EOPNOTSUPP; > > > >> >> + else if (file_out->f_op->copy_file_range != > > > >> >> +file_in->f_op->copy_file_range) > > > >> >> + return -EXDEV; > > > >> >> + } > > > >> > > > > >> > That looks strange, because you are duplicating the logic in > > > >> > do_copy_file_range(). Maybe better: > > > >> > > > > >> > if (WARN_ON_ONCE(flags & ~COPY_FILE_SPLICE)) > > > >> > return -EINVAL; > > > >> > if (flags & COPY_FILE_SPLICE) > > > >> >return do_splice_direct(file_in, _in, file_out, _out, > > > >> > len > MAX_RW_COUNT ? MAX_RW_COUNT : > > > >> > len, 0); > > > >> > > > >> My initial reasoning for duplicating the logic in do_copy_file_range() > > > >> was > > > >> to allow the generic_copy_file_range() callers to be left unmodified > > > >> and > > > >> allow the filesystems to default to this implementation. > > > >> > > > >> With this change, I guess that the calls to generic_copy_file_range() > > > >> from > > > >> the different filesystems can be dropped, as in my initial patch, as > > > >> they > > > >> will always get -EINVAL. The other option would be to set the > > > >> COPY_FILE_SPLICE flag in those calls, but that would get us back to the > > > >> problem we're trying to solve. > > > > > > > > I don't understand the problem. > > > > > > > > What exactly is wrong with the code I suggested? > > > > Why should any filesystem be changed? > > > > > > > > Maybe I am missing something. > > > > > > Ok, I have to do a full brain reboot and start all over. > > > > > > Before that, I picked the code you suggested and tested it. I've mounted > > > a cephfs filesystem and used xfs_io to execute a 'copy_range' command > > > using /sys/kernel/debug/sched_features as source. The result was a > > > 0-sized file in cephfs. And the reason is thevfs_copy_file_range() > > > early exit in: > > > > > > if (len == 0) > > > return 0; > > > > > > 'len'
Re: [PATCH 00/33] Network fs helper library & fscache kiocb API [ver #3]
On Mon, Feb 15, 2021 at 8:10 PM Matthew Wilcox wrote: > > On Mon, Feb 15, 2021 at 06:40:27PM -0600, Steve French wrote: > > It could be good if netfs simplifies the problem experienced by > > network filesystems on Linux with readahead on large sequential reads > > - where we don't get as much parallelism due to only having one > > readahead request at a time (thus in many cases there is 'dead time' > > on either the network or the file server while waiting for the next > > readpages request to be issued). This can be a significant > > performance problem for current readpages when network latency is long > > (or e.g. in cases when network encryption is enabled, and hardware > > offload not available so time consuming on the server or client to > > encrypt the packet). > > > > Do you see netfs much faster than currentreadpages for ceph? > > > > Have you been able to get much benefit from throttling readahead with > > ceph from the current netfs approach for clamping i/o? > > The switch from readpages to readahead does help in a couple of corner > cases. For example, if you have two processes reading the same file at > the same time, one will now block on the other (due to the page lock) > rather than submitting a mess of overlapping and partial reads. Do you have a simple repro example of this we could try (fio, dbench, iozone etc) to get some objective perf data? My biggest worry is making sure that the switch to netfs doesn't degrade performance (which might be a low bar now since current network file copy perf seems to signifcantly lag at least Windows), and in some easy to understand scenarios want to make sure it actually helps perf. -- Thanks, Steve
Re: [PATCH 00/33] Network fs helper library & fscache kiocb API [ver #3]
On Mon, Feb 15, 2021 at 8:10 PM Matthew Wilcox wrote: > > On Mon, Feb 15, 2021 at 06:40:27PM -0600, Steve French wrote: > > It could be good if netfs simplifies the problem experienced by > > network filesystems on Linux with readahead on large sequential reads > > - where we don't get as much parallelism due to only having one > > readahead request at a time (thus in many cases there is 'dead time' > > on either the network or the file server while waiting for the next > > readpages request to be issued). This can be a significant > > performance problem for current readpages when network latency is long > > (or e.g. in cases when network encryption is enabled, and hardware > > offload not available so time consuming on the server or client to > > encrypt the packet). > > > > Do you see netfs much faster than currentreadpages for ceph? > > > > Have you been able to get much benefit from throttling readahead with > > ceph from the current netfs approach for clamping i/o? > > The switch from readpages to readahead does help in a couple of corner > cases. For example, if you have two processes reading the same file at > the same time, one will now block on the other (due to the page lock) > rather than submitting a mess of overlapping and partial reads. > > We're not there yet on having multiple outstanding reads. Bill and I > had a chat recently about how to make the readahead code detect that > it is in a "long fat pipe" situation (as opposed to just dealing with > a slow device), and submit extra readahead requests to make best use of > the bandwidth and minimise blocking of the application. > > That's not something for the netfs code to do though; we can get into > that situation with highly parallel SSDs. This (readahead behavior improvements in Linux, on single large file sequential read workloads like cp or grep) gets particularly interesting with SMB3 as multichannel becomes more common. With one channel having one readahead request pending on the network is suboptimal - but not as bad as when multichannel is negotiated. Interestingly in most cases two network connections to the same server (different TCP sockets,but the same mount, even in cases where only network adapter) can achieve better performance - but still significantly lags Windows (and probably other clients) as in Linux we don't keep multiple I/Os in flight at one time (unless different files are being read at the same time by different threads). As network adapters are added and removed from the server (other client typically poll to detect interface changes, and SMB3 also leverages the "witness protocol" to get notification of adapter additions or removals) - it would be helpful to change the maximum number of readahead requests in flight. In addition, as the server throttles back (reducing the number of 'credits' granted to the client) it will be important to give hints to the readahead logic about reducing the number of read ahead requests in flight. Keeping multiple readahead requests is easier to imagine when multiple processes are copying or reading files, but there are many scenarios where we could do better with parallelizing a single process doing copy by ensuring that there is no 'dead time' on the network. -- Thanks, Steve
Re: [PATCH 00/33] Network fs helper library & fscache kiocb API [ver #3]
Jeff, What are the performance differences you are seeing (positive or negative) with ceph and netfs, especially with simple examples like file copy or grep of large files? It could be good if netfs simplifies the problem experienced by network filesystems on Linux with readahead on large sequential reads - where we don't get as much parallelism due to only having one readahead request at a time (thus in many cases there is 'dead time' on either the network or the file server while waiting for the next readpages request to be issued). This can be a significant performance problem for current readpages when network latency is long (or e.g. in cases when network encryption is enabled, and hardware offload not available so time consuming on the server or client to encrypt the packet). Do you see netfs much faster than currentreadpages for ceph? Have you been able to get much benefit from throttling readahead with ceph from the current netfs approach for clamping i/o? On Mon, Feb 15, 2021 at 12:08 PM Jeff Layton wrote: > > On Mon, 2021-02-15 at 15:44 +, David Howells wrote: > > Here's a set of patches to do two things: > > > > (1) Add a helper library to handle the new VM readahead interface. This > > is intended to be used unconditionally by the filesystem (whether or > > not caching is enabled) and provides a common framework for doing > > caching, transparent huge pages and, in the future, possibly fscrypt > > and read bandwidth maximisation. It also allows the netfs and the > > cache to align, expand and slice up a read request from the VM in > > various ways; the netfs need only provide a function to read a stretch > > of data to the pagecache and the helper takes care of the rest. > > > > (2) Add an alternative fscache/cachfiles I/O API that uses the kiocb > > facility to do async DIO to transfer data to/from the netfs's pages, > > rather than using readpage with wait queue snooping on one side and > > vfs_write() on the other. It also uses less memory, since it doesn't > > do buffered I/O on the backing file. > > > > Note that this uses SEEK_HOLE/SEEK_DATA to locate the data available > > to be read from the cache. Whilst this is an improvement from the > > bmap interface, it still has a problem with regard to a modern > > extent-based filesystem inserting or removing bridging blocks of > > zeros. Fixing that requires a much greater overhaul. > > > > This is a step towards overhauling the fscache API. The change is opt-in > > on the part of the network filesystem. A netfs should not try to mix the > > old and the new API because of conflicting ways of handling pages and the > > PG_fscache page flag and because it would be mixing DIO with buffered I/O. > > Further, the helper library can't be used with the old API. > > > > This does not change any of the fscache cookie handling APIs or the way > > invalidation is done. > > > > In the near term, I intend to deprecate and remove the old I/O API > > (fscache_allocate_page{,s}(), fscache_read_or_alloc_page{,s}(), > > fscache_write_page() and fscache_uncache_page()) and eventually replace > > most of fscache/cachefiles with something simpler and easier to follow. > > > > The patchset contains five parts: > > > > (1) Some helper patches, including provision of an ITER_XARRAY iov > > iterator and a function to do readahead expansion. > > > > (2) Patches to add the netfs helper library. > > > > (3) A patch to add the fscache/cachefiles kiocb API. > > > > (4) Patches to add support in AFS for this. > > > > (5) Patches from Jeff Layton to add support in Ceph for this. > > > > Dave Wysochanski also has patches for NFS for this, though they're not > > included on this branch as there's an issue with PNFS. > > > > With this, AFS without a cache passes all expected xfstests; with a cache, > > there's an extra failure, but that's also there before these patches. > > Fixing that probably requires a greater overhaul. Ceph and NFS also pass > > the expected tests. > > > > These patches can be found also on: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-netfs-lib > > > > For diffing reference, the tag for the 9th Feb pull request is > > fscache-ioapi-20210203 and can be found in the same repository. > > > > > > > > Changes > > === > > > > (v3) Rolled in the bug fixes. > > > > Adjusted the functions that unlock and wait for PG_fscache according > > to Linus's suggestion. > > > > Hold a ref on a page when PG_fscache is set as per Linus's > > suggestion. > > > > Dropped NFS support and added Ceph support. > > > > (v2) Fixed some bugs and added NFS support. > > > > > > References > > == > > > > These patches have been published for review before, firstly as part of a > > larger set: > > > > Link: > >
Re: [PATCH v2] vfs: prevent copy_file_range to copy across devices
On Mon, Feb 15, 2021 at 10:11 AM Trond Myklebust wrote: > > On Mon, 2021-02-15 at 15:43 +, Luis Henriques wrote: > > Nicolas Boichat reported an issue when trying to use the > > copy_file_range > > syscall on a tracefs file. It failed silently because the file > > content is > > generated on-the-fly (reporting a size of zero) and copy_file_range > > needs > > to know in advance how much data is present. > > That explanation makes no sense whatsoever. copy_file_range is a non- > atomic operation and so the file can change while being copied. Any > determination of 'how much data is present' that is made in advance > would therefore be a flaw in the copy process being used (i.e. > do_splice_direct()). Does sendfile() also 'issue' in the same way? I agree that the explanation of the tracefs problem motivating this patch doesn't make sense. -- Thanks, Steve
Re: [GIT PULL] cifs fixes
The branch also was mirrored to github so could be pulled from there if you prefer - see below (presumably the admins for samba.org, who live in Germany, are not online - and so I am not sure when samba.org servers will be restarted). The following changes since commit 92bf22614b21a2706f4993b278017e437f7785b3: Linux 5.11-rc7 (2021-02-07 13:57:38 -0800) are available in the Git repository at: git://github.com/smfrench/smb3-kernel.git tags/5.11-rc7-smb3-github for you to fetch changes up to a738c93fb1c17e386a09304b517b1c6b2a6a5a8b: cifs: Set CIFS_MOUNT_USE_PREFIX_PATH flag on setting cifs_sb->prepath. (2021-02-11 11:08:32 -0600) 4 small smb3 fixes to the new mount API Ronnie Sahlberg (3): cifs: fix dfs-links cifs: do not disable noperm if multiuser mount option is not provided cifs: In the new mount api we get the full devname as source= Shyam Prasad N (1): cifs: Set CIFS_MOUNT_USE_PREFIX_PATH flag on setting cifs_sb->prepath. fs/cifs/cifsfs.c | 2 +- fs/cifs/connect.c| 9 + fs/cifs/fs_context.c | 20 +--- fs/cifs/fs_context.h | 1 + 4 files changed, 28 insertions(+), 4 deletions(-) On Fri, Feb 12, 2021 at 2:05 PM Linus Torvalds wrote: > > On Fri, Feb 12, 2021 at 10:16 AM Steve French wrote: > > > > git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc7-smb3 > > It looks like git.samba.org is feeling very sick and is not answering. > Not git, not ping (but maybe icmp ping is blocked). > > Please give it a kick, or provide some other hosting mirror? > >Linus -- Thanks, Steve
Re: [GIT PULL] cifs fixes
Metze/Bjorn, Linus is right - samba.org is down for me (I also verified with JRA). Any ETA on when it gets back up? On Fri, Feb 12, 2021 at 2:05 PM Linus Torvalds wrote: > > On Fri, Feb 12, 2021 at 10:16 AM Steve French wrote: > > > > git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc7-smb3 > > It looks like git.samba.org is feeling very sick and is not answering. > Not git, not ping (but maybe icmp ping is blocked). > > Please give it a kick, or provide some other hosting mirror? > >Linus -- Thanks, Steve
[GIT PULL] cifs fixes
Please pull the following changes since commit 92bf22614b21a2706f4993b278017e437f7785b3: Linux 5.11-rc7 (2021-02-07 13:57:38 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc7-smb3 for you to fetch changes up to a738c93fb1c17e386a09304b517b1c6b2a6a5a8b: cifs: Set CIFS_MOUNT_USE_PREFIX_PATH flag on setting cifs_sb->prepath. (2021-02-11 11:08:32 -0600) 4 small cifs fixes for the implementation of the new mount API (including a particularly important one for DFS links) that were found in additional testing this week of additional DFS scenarios, and a user testing of an apache container problem. Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/501 Ronnie Sahlberg (3): cifs: fix dfs-links cifs: do not disable noperm if multiuser mount option is not provided cifs: In the new mount api we get the full devname as source= Shyam Prasad N (1): cifs: Set CIFS_MOUNT_USE_PREFIX_PATH flag on setting cifs_sb->prepath. fs/cifs/cifsfs.c | 2 +- fs/cifs/connect.c| 9 + fs/cifs/fs_context.c | 20 +--- fs/cifs/fs_context.h | 1 + 4 files changed, 28 insertions(+), 4 deletions(-) -- Thanks, Steve
[GIT PULL] cifs fixes
Please pull the following changes since commit 1048ba83fb1c00cd24172e23e8263972f6b5d9ac: Linux 5.11-rc6 (2021-01-31 13:50:09 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc6-smb3 for you to fetch changes up to 21b200d091826a83aafc95d847139b2b0582f6d1: cifs: report error instead of invalid when revalidating a dentry fails (2021-02-05 13:17:48 -0600) 3 small smb3 bug fixes for stable Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/491 Aurelien Aptel (1): cifs: report error instead of invalid when revalidating a dentry fails Gustavo A. R. Silva (1): smb3: Fix out-of-bounds bug in SMB2_negotiate() Pavel Shilovsky (1): smb3: fix crediting for compounding when only one request in flight fs/cifs/dir.c | 22 -- fs/cifs/smb2pdu.h | 2 +- fs/cifs/transport.c | 18 +++--- 3 files changed, 36 insertions(+), 6 deletions(-) -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed On Wed, Feb 3, 2021 at 4:59 AM Stephen Rothwell wrote: > > Hi all, > > Commit > > 37595a5c8a51 ("smb3: fix crediting for compounding when only one request in > flight") > > is missing a Signed-off-by from its committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: [PATCH] smb3: Fix out-of-bounds bug in SMB2_negotiate()
merged into cifs-2.6.git for-next On Mon, Feb 1, 2021 at 8:38 PM Gustavo A. R. Silva wrote: > > While addressing some warnings generated by -Warray-bounds, I found this > bug that was introduced back in 2017: > > CC [M] fs/cifs/smb2pdu.o > fs/cifs/smb2pdu.c: In function ‘SMB2_negotiate’: > fs/cifs/smb2pdu.c:822:16: warning: array subscript 1 is above array bounds > of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds] > 822 | req->Dialects[1] = cpu_to_le16(SMB30_PROT_ID); > | ~^~~ > fs/cifs/smb2pdu.c:823:16: warning: array subscript 2 is above array bounds > of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds] > 823 | req->Dialects[2] = cpu_to_le16(SMB302_PROT_ID); > | ~^~~ > fs/cifs/smb2pdu.c:824:16: warning: array subscript 3 is above array bounds > of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds] > 824 | req->Dialects[3] = cpu_to_le16(SMB311_PROT_ID); > | ~^~~ > fs/cifs/smb2pdu.c:816:16: warning: array subscript 1 is above array bounds > of ‘__le16[1]’ {aka ‘short unsigned int[1]’} [-Warray-bounds] > 816 | req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID); > | ~^~~ > > At the time, the size of array _Dialects_ was changed from 1 to 3 in struct > validate_negotiate_info_req, and then in 2019 it was changed from 3 to 4, > but those changes were never made in struct smb2_negotiate_req, which has > led to a 3 and a half years old out-of-bounds bug in function > SMB2_negotiate() (fs/cifs/smb2pdu.c). > > Fix this by increasing the size of array _Dialects_ in struct > smb2_negotiate_req to 4. > > Fixes: 9764c02fcbad ("SMB3: Add support for multidialect negotiate (SMB2.1 > and later)") > Fixes: d5c7076b772a ("smb3: add smb3.1.1 to default dialect list") > Cc: sta...@vger.kernel.org > Signed-off-by: Gustavo A. R. Silva > --- > fs/cifs/smb2pdu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h > index d85edf5d1429..a5a9e33c0d73 100644 > --- a/fs/cifs/smb2pdu.h > +++ b/fs/cifs/smb2pdu.h > @@ -286,7 +286,7 @@ struct smb2_negotiate_req { > __le32 NegotiateContextOffset; /* SMB3.1.1 only. MBZ earlier */ > __le16 NegotiateContextCount; /* SMB3.1.1 only. MBZ earlier */ > __le16 Reserved2; > - __le16 Dialects[1]; /* One dialect (vers=) at a time for now */ > + __le16 Dialects[4]; /* BB expand this if autonegotiate > 4 dialects */ > } __packed; > > /* Dialects */ > -- > 2.27.0 > -- Thanks, Steve
[GIT] cifs fixes
Please pull the following changes since commit 6ee1d745b7c9fd573fba142a2efdad76a9f1cb04: Linux 5.11-rc5 (2021-01-24 16:47:14 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc5-smb3 for you to fetch changes up to 0d4873f9aa4ff8fc1d63a5755395b794d32ce046: cifs: fix dfs domain referrals (2021-01-28 21:40:43 -0600) Four cifs patches found in additional testing of the conversion to the new mount API. 3 small parm processing ones, and one fixing domain based DFS referrals A fix for nested mounts, an additional DFS fix and a security fix are still being tested (I expect to send those next week). Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/486 Adam Harvey (1): cifs: ignore auto and noauto options if given Ronnie Sahlberg (1): cifs: fix dfs domain referrals Steve French (2): cifs: fix mounts to subdirectories of target cifs: returning mount parm processing errors correctly fs/cifs/cifs_dfs_ref.c | 12 fs/cifs/cifsfs.c | 2 +- fs/cifs/cifsproto.h| 6 -- fs/cifs/connect.c | 32 ++-- fs/cifs/dfs_cache.c| 8 +--- fs/cifs/fs_context.c | 41 + 6 files changed, 81 insertions(+), 20 deletions(-) -- Thanks, Steve
regression in drm_blank.c?
For the last month my logs have been flooded many times a second with: "nouveau :01:00.0: [drm] *ERROR* crtc 50: Can't calculate constants, dotclock = 0!" (see line 641 of drm_vblank.c) which is distracting for debugging all other kernel problems (since dmesg entries on this system are 99+% this message). Am running current kernel ie 5.11-rc5 (although it also was very very noisty in 5.10), Ubuntu mainline kernel builds, Lenovo P52 Thinkpad laptop. Could it be due to one of the more recent changes to drivers/gpu/drm/drm_vblank.c? Ideas how to workaround this? -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit 19c329f6808995b142b3966301f217c831e7cf31: Linux 5.11-rc4 (2021-01-17 16:37:05 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc4-smb3 for you to fetch changes up to 214a5ea081e77346e4963dd6d20c5539ff8b6ae6: cifs: do not fail __smb_send_rqst if non-fatal signals are pending (2021-01-23 01:28:20 -0600) an important signal handling patch for stable, and two small cleanup patches Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/485 Jiapeng Zhong (2): fs/cifs: Assign boolean values to a bool variable fs/cifs: Simplify bool comparison. Ronnie Sahlberg (1): cifs: do not fail __smb_send_rqst if non-fatal signals are pending fs/cifs/connect.c | 4 ++-- fs/cifs/transport.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) -- Thanks, Steve
Re: [PATCH] fs/cifs: Replace one-element array with flexible-array member.
On Sun, Jan 17, 2021 at 6:02 PM Al Viro wrote: > > On Sun, Jan 17, 2021 at 03:58:23PM -0600, Steve French wrote: > > Jiapeng, > > Aurelien is correct, you should respin this patch and correct for > > where it breaks the sizeof calculation. For example your change: > > > > struct smb2_lock_rsp { > > @@ -1434,7 +1434,7 @@ struct smb2_query_directory_req { > > __le16 FileNameOffset; > > __le16 FileNameLength; > > __le32 OutputBufferLength; > > - __u8 Buffer[1]; > > + __u8 Buffer[]; > > } __packed; > > > > would have the side effect of making the file name off by one: > > > > smb2pdu.c-4654- req->FileNameOffset = > > smb2pdu.c:4655: cpu_to_le16(sizeof(struct > > smb2_query_directory_req) - 1); > > FWIW, that sizeof() - 1 should've been offsetof()... agreed -- Thanks, Steve
Re: [PATCH] fs/cifs: Replace one-element array with flexible-array member.
Jiapeng, Aurelien is correct, you should respin this patch and correct for where it breaks the sizeof calculation. For example your change: struct smb2_lock_rsp { @@ -1434,7 +1434,7 @@ struct smb2_query_directory_req { __le16 FileNameOffset; __le16 FileNameLength; __le32 OutputBufferLength; - __u8 Buffer[1]; + __u8 Buffer[]; } __packed; would have the side effect of making the file name off by one: smb2pdu.c-4654- req->FileNameOffset = smb2pdu.c:4655: cpu_to_le16(sizeof(struct smb2_query_directory_req) - 1); On Thu, Jan 14, 2021 at 3:26 AM Aurélien Aptel via samba-technical wrote: > > Hi Jiapeng, > > This will change the size returned by sizeof(). Have you checked that > this doesn't introduce off-by-one errors in all the sizeof() usage? > > Cheers, > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) > > -- Thanks, Steve
Re: [PATCH v2] fs/cifs: Simplify bool comparison.
The patch wouldn't merge (probably fixed with an earlier patch) so I removed the second part of it (ie the change around line 3740) - see below and merged the smaller change below into cifs-2.6.git for-next commit d1639d92fc762bf80273aaf52d87eb780711714c (HEAD -> for-next, origin/for-next) Author: Jiapeng Zhong Date: Thu Jan 14 18:02:23 2021 +0800 fs/cifs: Simplify bool comparison. Fix the follow warnings: ./fs/cifs/connect.c: WARNING: Comparison of 0/1 to bool variable Reported-by: Abaci Robot Signed-off-by: Jiapeng Zhong Signed-off-by: Steve French diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 7c3325c0fadc..c8ef24bac94f 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2628,7 +2628,7 @@ void reset_cifs_unix_caps(unsigned int xid, struct cifs_tcon *tcon, } else if (ctx) tcon->unix_ext = 1; /* Unix Extensions supported */ - if (tcon->unix_ext == 0) { + if (!tcon->unix_ext) { cifs_dbg(FYI, "Unix extensions disabled so not set on reconnect\n"); return; } On Thu, Jan 14, 2021 at 4:07 AM Jiapeng Zhong wrote: > > Fix the follow warnings: > > ./fs/cifs/connect.c: WARNING: Comparison of 0/1 to bool variable > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Zhong > > --- > Changes in v2: > -Modified subject. > > fs/cifs/connect.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index b9df855..b7869a2 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -2628,7 +2628,7 @@ void reset_cifs_unix_caps(unsigned int xid, struct > cifs_tcon *tcon, > } else if (ctx) > tcon->unix_ext = 1; /* Unix Extensions supported */ > > - if (tcon->unix_ext == 0) { > + if (!tcon->unix_ext) { > cifs_dbg(FYI, "Unix extensions disabled so not set on > reconnect\n"); > return; > } > @@ -3740,7 +3740,7 @@ static void delayed_free(struct rcu_head *p) > > if (!ses->binding) { > ses->capabilities = server->capabilities; > - if (linuxExtEnabled == 0) > + if (!linuxExtEnabled) > ses->capabilities &= (~server->vals->cap_unix); > > if (ses->auth_key.response) { > -- > 1.8.3.1 > -- Thanks, Steve
Re: [PATCH] fs/cifs: Assign boolean values to a bool variable
merged into cifs-2.6.git for-next On Thu, Jan 14, 2021 at 3:11 AM Jiapeng Zhong wrote: > > Fix the following coccicheck warnings: > > ./fs/cifs/connect.c:3386:2-21: WARNING: Assignment of 0/1 to > bool variable. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Zhong > --- > fs/cifs/connect.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index b9df855..8fbb5ea 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -2195,7 +2195,7 @@ static int match_tcon(struct cifs_tcon *tcon, struct > smb3_fs_context *ctx) > if (ses->server->capabilities & SMB2_GLOBAL_CAP_DIRECTORY_LEASING) > tcon->nohandlecache = ctx->nohandlecache; > else > - tcon->nohandlecache = 1; > + tcon->nohandlecache = true; > tcon->nodelete = ctx->nodelete; > tcon->local_lease = ctx->local_lease; > INIT_LIST_HEAD(>pending_opens); > -- > 1.8.3.1 > -- Thanks, Steve
Re: [PATCH] cifs: fix msleep() is imprecise
This patch seems reasonable at first glance, but I was a little concerned that we don't see many users yet of fsleep. Has there been pushback on converting "yield" situations from using msleep to fsleep? On Thu, Dec 10, 2020 at 3:09 AM Yejune Deng wrote: > > See Documentation/timers/timers-howto.rst, msleep() is not > for (1ms - 20ms), There is a more advanced API is used. > > Signed-off-by: Yejune Deng > --- > fs/cifs/cifsfs.c| 4 ++-- > fs/cifs/connect.c | 14 +++--- > fs/cifs/file.c | 6 +++--- > fs/cifs/smbdirect.c | 2 +- > 4 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index 472cb77..d35ce52 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -664,10 +664,10 @@ static void cifs_umount_begin(struct super_block *sb) > cifs_dbg(FYI, "wake up tasks now - umount begin not > complete\n"); > wake_up_all(>ses->server->request_q); > wake_up_all(>ses->server->response_q); > - msleep(1); /* yield */ > + fsleep(1000); /* yield */ > /* we have to kick the requests once more */ > wake_up_all(>ses->server->response_q); > - msleep(1); > + fsleep(1000); > } > > return; > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 44f9cce..62a9c64 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -538,7 +538,7 @@ static inline int reconn_setup_dfs_targets(struct > cifs_sb_info *cifs_sb, > if (rc) { > cifs_dbg(FYI, "reconnect error %d\n", rc); > mutex_unlock(>srv_mutex); > - msleep(3000); > + ssleep(3); > } else { > atomic_inc(); > set_credits(server, 1); > @@ -621,7 +621,7 @@ static inline int reconn_setup_dfs_targets(struct > cifs_sb_info *cifs_sb, > server->bigbuf = (char *)cifs_buf_get(); > if (!server->bigbuf) { > cifs_server_dbg(VFS, "No memory for large SMB > response\n"); > - msleep(3000); > + ssleep(3); > /* retry will check if exiting */ > return false; > } > @@ -634,7 +634,7 @@ static inline int reconn_setup_dfs_targets(struct > cifs_sb_info *cifs_sb, > server->smallbuf = (char *)cifs_small_buf_get(); > if (!server->smallbuf) { > cifs_server_dbg(VFS, "No memory for SMB response\n"); > - msleep(1000); > + ssleep(1); > /* retry will check if exiting */ > return false; > } > @@ -729,7 +729,7 @@ static inline int reconn_setup_dfs_targets(struct > cifs_sb_info *cifs_sb, > * to clear and app threads to set tcpStatus > * CifsNeedReconnect if server hung. > */ > - usleep_range(1000, 2000); > + fsleep(1000); > length = 0; > continue; > } > @@ -790,7 +790,7 @@ static inline int reconn_setup_dfs_targets(struct > cifs_sb_info *cifs_sb, > */ > cifs_dbg(FYI, "RFC 1002 negative session response\n"); > /* give server a second to clean up */ > - msleep(1000); > + ssleep(1); > /* > * Always try 445 first on reconnect since we get NACK > * on some if we ever connected to port 139 (the NACK > @@ -944,7 +944,7 @@ static void clean_demultiplex_info(struct TCP_Server_Info > *server) > * response and going ahead and killing cifsd. > */ > cifs_dbg(FYI, "Wait for exit from demultiplex thread\n"); > - msleep(46000); > + ssleep(46); > /* > * If threads still have not exited they are probably never > * coming home not much else we can do but free the memory. > @@ -3655,7 +3655,7 @@ static void rfc1002mangle(char *target, char *source, > unsigned int length) > * significant slowing down on mount > * for everyone else > */ > - usleep_range(1000, 2000); > + fsleep(1000); > } > /* > * else the negprot may still work without this > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index be46fab..75538a8 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -283,7 +283,7 @@ int cifs_posix_open(char *full_path, struct inode > **pinode, > cifs_down_write(struct rw_semaphore *sem) > { > while (!down_write_trylock(sem)) > -
Re: [PATCH] cifs: check pointer before freeing
merged into cifs-2.6.git for-next and added cc:stable On Tue, Jan 5, 2021 at 5:08 PM Nathan Chancellor wrote: > > On Tue, Jan 05, 2021 at 12:21:26PM -0800, t...@redhat.com wrote: > > From: Tom Rix > > > > clang static analysis reports this problem > > > > dfs_cache.c:591:2: warning: Argument to kfree() is a constant address > > (18446744073709551614), which is not memory allocated by malloc() > > kfree(vi); > > ^ > > > > In dfs_cache_del_vol() the volume info pointer 'vi' being freed > > is the return of a call to find_vol(). The large constant address > > is find_vol() returning an error. > > > > Add an error check to dfs_cache_del_vol() similar to the one done > > in dfs_cache_update_vol(). > > > > Fixes: 54be1f6c1c37 ("cifs: Add DFS cache routines") > > Signed-off-by: Tom Rix > > Reviewed-by: Nathan Chancellor > > > --- > > fs/cifs/dfs_cache.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c > > index 6ad6ba5f6ebe..0fdb0de7ff86 100644 > > --- a/fs/cifs/dfs_cache.c > > +++ b/fs/cifs/dfs_cache.c > > @@ -1260,7 +1260,8 @@ void dfs_cache_del_vol(const char *fullpath) > > vi = find_vol(fullpath); > > spin_unlock(_list_lock); > > > > - kref_put(>refcnt, vol_release); > > + if (!IS_ERR(vi)) > > + kref_put(>refcnt, vol_release); > > } > > > > /** > > -- > > 2.27.0 > > -- Thanks, Steve
Re: [PATCH] cifs: style: replace one-element array with flexible-array
merged into cifs-2.6.git for-next On Wed, Dec 30, 2020 at 12:37 AM YANG LI wrote: > > There is a regular need in the kernel to provide a way to declare > having a dynamically sized set of trailing elements in a structure. > Kernel code should always use "flexible array members"[1] for these > cases. The older style of one-element or zero-length arrays should > no longer be used[2]. > > [1] https://en.wikipedia.org/wiki/Flexible_array_member > [2] https://www.kernel.org/doc/html/v5.9/process/ > deprecated.html#zero-length-and-one-element-arrays > > Signed-off-by: YANG LI > Reported-by: Abaci > --- > fs/cifs/smb2pdu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h > index 204a622..d85edf5 100644 > --- a/fs/cifs/smb2pdu.h > +++ b/fs/cifs/smb2pdu.h > @@ -424,7 +424,7 @@ struct smb2_rdma_transform_capabilities_context { > __le16 TransformCount; > __u16 Reserved1; > __u32 Reserved2; > - __le16 RDMATransformIds[1]; > + __le16 RDMATransformIds[]; > } __packed; > > /* Signing algorithms */ > -- > 1.8.3.1 > -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit e13300bdaa68f5487000e66baed1ff69bcb510bf: Merge tag '5.11-rc-smb3' of git://git.samba.org/sfrench/cifs-2.6 (2020-12-17 17:41:37 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc-smb3-part2 for you to fetch changes up to 9541b81322e60120b299222919957becd7a13683: Add SMB 2 support for getting and setting SACLs (2020-12-18 23:32:04 -0600) Four small CIFS/SMB3 fixes (witness protocol and reconnect related), and two that add ability to get and set auditing information in the security descriptor (SACL), which can be helpful not just for backup scenarios ("smbinfo secdesc" etc.) but also for improving security. Testing results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/466 Boris Protopopov (2): SMB3: Add support for getting and setting SACLs Add SMB 2 support for getting and setting SACLs Dan Carpenter (3): cifs: Delete a stray unlock in cifs_swn_reconnect() cifs: Unlock on errors in cifs_swn_reconnect() cifs: Re-indent cifs_swn_reconnect() Samuel Cabrero (1): cifs: Avoid error pointer dereference fs/cifs/cifs_swn.c | 73 +++ fs/cifs/cifsacl.c | 15 ++ fs/cifs/cifsglob.h | 4 +-- fs/cifs/cifspdu.h | 2 ++ fs/cifs/cifsproto.h | 4 +-- fs/cifs/connect.c | 1 + fs/cifs/smb2ops.c | 35 ++- fs/cifs/smb2pdu.c | 7 +++-- fs/cifs/smb2proto.h | 4 +-- fs/cifs/xattr.c | 81 +++-- 10 files changed, 140 insertions(+), 86 deletions(-) -- Thanks, Steve
Re: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs
/fs/cifs/xattr.c > +++ b/fs/cifs/xattr.c > @@ -340,21 +340,19 @@ static int cifs_xattr_get(const struct xattr_handler > *handler, > * fetch owner, DACL, and SACL if asked for full descriptor, > * fetch owner and DACL otherwise > */ > - u32 acllen, additional_info = 0; > + u32 acllen, extra_info; > struct cifs_ntsd *pacl; > > if (pTcon->ses->server->ops->get_acl == NULL) > goto out; /* rc already EOPNOTSUPP */ > > if (handler->flags == XATTR_CIFS_NTSD_FULL) { > - additional_info = OWNER_SECINFO | GROUP_SECINFO | > - DACL_SECINFO | SACL_SECINFO; > + extra_info = SACL_SECINFO; > } else { > - additional_info = OWNER_SECINFO | GROUP_SECINFO | > - DACL_SECINFO; > + extra_info = 0; > } > pacl = pTcon->ses->server->ops->get_acl(cifs_sb, > - inode, full_path, , additional_info); > + inode, full_path, , extra_info); > if (IS_ERR(pacl)) { > rc = PTR_ERR(pacl); > cifs_dbg(VFS, "%s: error %zd getting sec desc\n", > -- > 2.18.4 > -- Thanks, Steve From d42d0c0334ef918310b11818c1805624c0ee9440 Mon Sep 17 00:00:00 2001 From: Boris Protopopov Date: Fri, 18 Dec 2020 11:30:12 -0600 Subject: [PATCH 1/2] SMB3: Add support for getting and setting SACLs Add SYSTEM_SECURITY access flag and use with smb2 when opening files for getting/setting SACLs. Add "system.cifs_ntsd_full" extended attribute to allow user-space access to the functionality. Avoid multiple server calls when setting owner, DACL, and SACL. Signed-off-by: Boris Protopopov Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 15 fs/cifs/cifsglob.h | 4 +-- fs/cifs/cifspdu.h | 2 ++ fs/cifs/cifsproto.h | 4 +-- fs/cifs/smb2ops.c | 31 ++--- fs/cifs/smb2pdu.c | 5 ++- fs/cifs/smb2proto.h | 4 +-- fs/cifs/xattr.c | 83 + 8 files changed, 100 insertions(+), 48 deletions(-) diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 2f21f89871cc..2c9051f5e86a 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -1195,7 +1195,8 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, } struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb, - const struct cifs_fid *cifsfid, u32 *pacllen) + const struct cifs_fid *cifsfid, u32 *pacllen, + u32 __maybe_unused unused) { struct cifs_ntsd *pntsd = NULL; unsigned int xid; @@ -1263,7 +1264,7 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, /* Retrieve an ACL from the server */ struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb, struct inode *inode, const char *path, - u32 *pacllen) + u32 *pacllen, u32 info) { struct cifs_ntsd *pntsd = NULL; struct cifsFileInfo *open_file = NULL; @@ -1273,7 +1274,7 @@ struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb, if (!open_file) return get_cifs_acl_by_path(cifs_sb, path, pacllen); - pntsd = get_cifs_acl_by_fid(cifs_sb, _file->fid, pacllen); + pntsd = get_cifs_acl_by_fid(cifs_sb, _file->fid, pacllen, info); cifsFileInfo_put(open_file); return pntsd; } @@ -1338,6 +1339,7 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr, int rc = 0; struct tcon_link *tlink = cifs_sb_tlink(cifs_sb); struct smb_version_operations *ops; + const u32 info = 0; cifs_dbg(NOISY, "converting ACL to mode for %s\n", path); @@ -1347,9 +1349,9 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr, ops = tlink_tcon(tlink)->ses->server->ops; if (pfid && (ops->get_acl_by_fid)) - pntsd = ops->get_acl_by_fid(cifs_sb, pfid, ); + pntsd = ops->get_acl_by_fid(cifs_sb, pfid, , info); else if (ops->get_acl) - pntsd = ops->get_acl(cifs_sb, inode, path, ); + pntsd = ops->get_acl(cifs_sb, inode, path, , info); else { cifs_put_tlink(tlink); return -EOPNOTSUPP; @@ -1388,6 +1390,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, struct tcon_link *tlink = cifs_sb_tlink(cifs_sb); struct smb_version_operations *ops; bool mode_from_sid, id_from_sid; + const u32 info = 0; if (IS_ERR(tlink)) return PTR_ERR(tlink); @@ -1403,7 +1406,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, return -EOPNOTSUPP; } - pntsd = ops->get_acl(cifs_sb, inode, path, ); + pntsd = ops->get_acl(cifs_sb, inode
Re: linux-next: Signed-off-by missing for commit in the cifs tree
I have now amended the patches to have the correct author and repushed On Thu, Dec 17, 2020 at 11:57 PM Stephen Rothwell wrote: > > Hi all, > > Commits > > ee04fe87f4d0 ("cifs: Re-indent cifs_swn_reconnect()") > 2c6dd2f9742d ("cifs: Unlock on errors in cifs_swn_reconnect()") > fb2356b3739b ("cifs: Delete a stray unlock in cifs_swn_reconnect()") > > are missing a Signed-off-by from their author. > > This is the problem with the mailing list again :-( The > author has been recorded as > > "Dan Carpenter via samba-technical " > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes and Improvements for 5.11-rc
Please pull the following changes since commit 2c85ebc57b3e1817b6ce1a6b703928e113a90442: Linux 5.10 (2020-12-13 14:41:30 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc-smb3 for you to fetch changes up to afee4410bc6c50e1422c5a45d633ad0e478ea960: cifs: update internal module version number (2020-12-16 21:56:42 -0600) cifs/smb3 changes: the largest part are for support of the newer mount API which has been needed for cifs/smb3 mounts for a long time due to the new API's better handling of remount, and better error reporting. There are three additional small cleanup patches for this being tested, that are not included yet. This series also includes addition of support for the SMB3 witness protocol which can provide important notifications from the server to client on server address or export or network changes. This can be useful for example in order to be notified before the failure - when a server's IP address changes (in the future it will allow us to support server notifications of when a share is moved). It also includes three patches for stable e.g. some that better handle some confusing error messages during session establishment. Testing results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/462 There are three small patches from Dan Carpenter and also some unrelated (ie unrelated to this PR) multichannel and GCM256 (SMB3 encryption) fixes that are being investigated that are not included in this PR but I hope to finish off reasonably soon. Note that this series has a trivial merge conflict with a recent treewide change which Stephen Rothwell resolved in linux-next with a small patch: [PATCH] fixup for "treewide: rename nla_strlcpy to nla_strscpy." If you need me to send that or add it to my tree let me know. Dmitry Osipenko (1): cifs: Add missing sentinel to smb3_fs_parameters Gustavo A. R. Silva (1): cifs: Fix fall-through warnings for Clang Ronnie Sahlberg (23): cifs: rename smb_vol as smb3_fs_context and move it to fs_context.h cifs: rename dup_vol to smb3_fs_context_dup and move it into fs_context.c cifs: move the enum for cifs parameters into fs_context.h cifs: move cifs_parse_devname to fs_context.c cifs: switch to new mount api cifs: remove the devname argument to cifs_compose_mount_options cifs: add an smb3_fs_context to cifs_sb cifs: get rid of cifs_sb->mountdata cifs: remove [gu]id/backup[gu]id/file_mode/dir_mode from cifs_sb cifs: remove actimeo from cifs_sb cifs: move cifs_cleanup_volume_info[_content] to fs_context.c cifs: move [brw]size from cifs_sb to cifs_sb->ctx cifs: add initial reconfigure support cifs: we do not allow changing username/password/unc/... during remount cifs: simplify handling of cifs_sb/ctx->local_nls cifs: don't create a temp nls in cifs_setup_ipc cifs: uncomplicate printing the iocharset parameter cifs: do not allow changing posix_paths during remount cifs: remove ctx argument from cifs_setup_cifs_sb cifs: move update of flags into a separate function cifs: update mnt_cifs_flags during reconfigure cifs: fix uninitialized variable in smb3_fs_context_parse_param cifs: fix use after free in cifs_smb3_do_mount() Samuel Cabrero (11): cifs: Make extract_hostname function public cifs: Make extract_sharename function public cifs: Register generic netlink family cifs: add witness mount option and data structs cifs: Send witness register and unregister commands to userspace daemon cifs: Set witness notification handler for messages from userspace daemon cifs: Add witness information to debug data dump cifs: Send witness register messages to userspace daemon in echo task cifs: Simplify reconnect code when dfs upcall is enabled cifs: Handle witness client move notification cifs: Fix some error pointers handling detected by static checker Shyam Prasad N (3): cifs: Fix unix perm bits to cifsacl conversion for "other" bits. cifs: Enable sticky bit with cifsacl mount option. cifs: Tracepoints and logs for tracing credit changes. Steve French (16): SMB3: avoid confusing warning message on mount to Azure SMB3.1.1: remove confusing mount warning when no SPNEGO info on negprot rsp SMB3.1.1: update comments clarifying SPNEGO info in negprot response SMB3.1.1: do not log warning message if server doesn't populate salt cifs: minor kernel style fixes for comments cifs: cleanup misc.c cifs: minor updates to Kconfig cifs: remove various function description warnings cifs: remove some minor warnings pointed out by kernel test robot
Re: [EXTERNAL] Re: linux-next: build failure after merge of the net-next tree
On Wed, Dec 16, 2020 at 4:08 AM Stephen Rothwell wrote: > > Hi Steven, > > On Wed, 16 Dec 2020 02:21:26 + Steven French > wrote: > > > > I applied your patch to the tip of my tree (cifs-2.6.git for-next) - > > hopefully that makes it easier when I sent the PR in a day or two for > > cifs/smb3 changes. > > I think you have just made your tree fail to build as nla_strscpy does > not exist in your tree ... Just remove that commit and tell Linus about > the necessary change and he can add it to the merge. Done. Removed. -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed On Mon, Dec 14, 2020 at 1:32 PM Stephen Rothwell wrote: > > Hi all, > > Commits > > 425f40482491 ("cifs: fix uninitialized variable in > smb3_fs_context_parse_param") > 8cc9a66ea70e ("cifs: update mnt_cifs_flags during reconfigure") > > are missing a Signed-off-by from their committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed and repushed to cifs-2.6.git for-next On Sun, Dec 13, 2020 at 4:14 AM Stephen Rothwell wrote: > > Hi all, > > Commit > > 3aa4e616197b ("cifs: Register generic netlink family") > > is missing a Signed-off-by from its committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed and repushed to cifs-2.6.git for-next On Thu, Dec 10, 2020 at 4:45 AM Stephen Rothwell wrote: > > Hi all, > > Commit > > 7e9c48d305d5 ("cifs: remove the devname argument to > cifs_compose_mount_options") > > is missing a Signed-off-by from its committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commits in the cifs tree
Fixed - thx On Wed, Dec 9, 2020 at 3:23 PM Stephen Rothwell wrote: > > Hi all, > > Commits > > d24e661920cb ("cifs: Enable sticky bit with cifsacl mount option.") > 4bcfb51f82f0 ("cifs: Fix unix perm bits to cifsacl conversion for "other" > bits.") > > are missing a Signed-off-by from their committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: [PATCH 003/141] cifs: Fix fall-through warnings for Clang
Merged into cifs-2.6.git for-next Let me know if you see any other cleanup/misc cifs.ko patches that may have gotten missed ... On Fri, Nov 20, 2020 at 12:25 PM Gustavo A. R. Silva wrote: > > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple > warnings by explicitly adding multiple break/goto statements instead of > just letting the code fall through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > fs/cifs/inode.c | 1 + > fs/cifs/sess.c | 1 + > fs/cifs/smbdirect.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 9ee5f304592f..ac01f9684b39 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -771,6 +771,7 @@ cifs_get_file_info(struct file *filp) > */ > rc = 0; > CIFS_I(inode)->time = 0; > + goto cgfi_exit; > default: > goto cgfi_exit; > } > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index de564368a887..6c2c42f8d893 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -812,6 +812,7 @@ cifs_select_sectype(struct TCP_Server_Info *server, enum > securityEnum requested) > return NTLMv2; > if (global_secflags & CIFSSEC_MAY_NTLM) > return NTLM; > + break; > default: > break; > } > diff --git a/fs/cifs/smbdirect.c b/fs/cifs/smbdirect.c > index b029ed31ef91..10dfe5006792 100644 > --- a/fs/cifs/smbdirect.c > +++ b/fs/cifs/smbdirect.c > @@ -246,6 +246,7 @@ smbd_qp_async_error_upcall(struct ib_event *event, void > *context) > case IB_EVENT_CQ_ERR: > case IB_EVENT_QP_FATAL: > smbd_disconnect_rdma_connection(info); > + break; > > default: > break; > -- > 2.27.0 > -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit 509a15421674b9e1a3e1916939d0d0efd3e578da: Merge tag '5.10-rc6-smb3-fixes' of git://git.samba.org/sfrench/cifs-2.6 (2020-12-01 15:43:53 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.10-rc6-smb3-fixes-part2 for you to fetch changes up to ea64370bcae126a88cd26a16f1abcc23ab2b9a55: cifs: refactor create_sd_buf() and and avoid corrupting the buffer (2020-12-03 17:12:14 -0600) Three smb3 fixes (two for stable) addressing - a null pointer issue in a DFS error path - a problem with excessive padding when mounted with "idsfromsid" causing owner fields to get corrupted - a more recent problem with compounded reparse point query found in testing to the Linux kernel server Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/445 Aurelien Aptel (1): cifs: add NULL check for ses->tcon_ipc Namjae Jeon (1): smb3: set COMPOUND_FID to FileID field of subsequent compound request Ronnie Sahlberg (1): cifs: refactor create_sd_buf() and and avoid corrupting the buffer fs/cifs/connect.c | 3 ++- fs/cifs/smb2ops.c | 4 ++-- fs/cifs/smb2pdu.c | 71 +-- fs/cifs/smb2pdu.h | 2 -- 4 files changed, 42 insertions(+), 38 deletions(-) -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit b65054597872ce3aefbc6a666385eabdf9e288da: Linux 5.10-rc6 (2020-11-29 15:50:50 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.10-rc6-smb3-fixes for you to fetch changes up to 212253367dc7b49ed3fc194ce71b0992eacaecf2: cifs: fix potential use-after-free in cifs_echo_request() (2020-11-30 15:23:45 -0600) Two smb3 fixes for stable including a use after free fix, and one for signal handling in read. Build verification test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/441 Paulo Alcantara (2): cifs: allow syscalls to be restarted in __smb_send_rqst() cifs: fix potential use-after-free in cifs_echo_request() fs/cifs/connect.c | 2 ++ fs/cifs/transport.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit 09162bc32c880a791c6c0668ce0745cf7958f576: Linux 5.10-rc4 (2020-11-15 16:44:31 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.10-rc5-smb3-fixes for you to fetch changes up to 1254100030b3377e8302f9c75090ab191d73ee7c: smb3: Handle error case during offload read path (2020-11-15 23:05:33 -0600) Four smb3 fixes for stable: one fixes a memleak, the other three address a problem found with decryption offload that can cause a use after free. This PR was delayed due to changes to the cifs/smb3 testing infrastructure (multiple testing configuration changes and improvements). There are a few additional cifs/smb3 fixes in progress that can now be finished and tested that I will send with later PR. Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/437 Namjae Jeon (1): cifs: fix a memleak with modefromsid Rohith Surabattula (3): smb3: Call cifs reconnect from demultiplex thread smb3: Avoid Mid pending list corruption smb3: Handle error case during offload read path fs/cifs/cifsacl.c | 1 + fs/cifs/smb2ops.c | 88 +-- 2 files changed, 74 insertions(+), 15 deletions(-) -- Thanks, Steve
Ubuntu mainline kernel builds now failing not able to find module.lds file
I typically build cifs.ko for testing using the latest Ubuntu mainline build - but building a module in the 5.10-rc1 kernel - while booted to the 5.10-rc1 ubuntu mainlinekerel - e.g. "make C=1 -C /usr/src/linux-headers-`uname -r` M=`pwd` modules CF=-D__CHECK_ENDIAN__" which has worked for years - no longer works. make: Entering directory '/usr/src/linux-headers-5.10.0-051000rc1-generic' make[2]: *** No rule to make target 'scripts/module.lds', needed by '/home/smfrench/cifs-2.6/fs/cifs/cifs.ko'. Stop. make[1]: *** [scripts/Makefile.modpost:117: __modpost] Error 2 make: *** [Makefile:1703: modules] Error 2 make: Leaving directory '/usr/src/linux-headers-5.10.0-051000rc1-generic' I don't see a file in scripts/module.lds in /usr/src/linux-headers-5.10.0-051000rc1-generic/scripts directory copying from scripts/module.lds in the 5.10-rc1 git tree to /usr/src/linux-headers-5.10.0-051000rc1-generic/scripts fixed the problem but was wondering if this is just a packaging problem with Ubuntu (missing a file in the kernel headers package for their mainline daily builds?) -- Thanks, Steve
Re: [PATCH v2 1/2] cifs: convert to add_to_page_cache()
you can add my reviewed-by if you would like On Thu, Oct 22, 2020 at 1:48 AM Kent Overstreet wrote: > > This is just open coding add_to_page_cache(), and the next patch will > delete add_to_page_cache_locked(). > > Signed-off-by: Kent Overstreet > --- > fs/cifs/file.c | 20 > 1 file changed, 4 insertions(+), 16 deletions(-) > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index be46fab4c9..b3ee790532 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -4296,20 +4296,11 @@ readpages_get_pages(struct address_space *mapping, > struct list_head *page_list, > > page = lru_to_page(page_list); > > - /* > -* Lock the page and put it in the cache. Since no one else > -* should have access to this page, we're safe to simply set > -* PG_locked without checking it first. > -*/ > - __SetPageLocked(page); > - rc = add_to_page_cache_locked(page, mapping, > - page->index, gfp); > + rc = add_to_page_cache(page, mapping, page->index, gfp); > > /* give up if we can't stick it in the cache */ > - if (rc) { > - __ClearPageLocked(page); > + if (rc) > return rc; > - } > > /* move first page to the tmplist */ > *offset = (loff_t)page->index << PAGE_SHIFT; > @@ -4328,12 +4319,9 @@ readpages_get_pages(struct address_space *mapping, > struct list_head *page_list, > if (*bytes + PAGE_SIZE > rsize) > break; > > - __SetPageLocked(page); > - rc = add_to_page_cache_locked(page, mapping, page->index, > gfp); > - if (rc) { > - __ClearPageLocked(page); > + rc = add_to_page_cache(page, mapping, page->index, gfp); > + if (rc) > break; > - } > list_move_tail(>lru, tmplist); > (*bytes) += PAGE_SIZE; > expected_index++; > -- > 2.28.0 > -- Thanks, Steve
Re: [PATCH 1/2] cifs: convert to add_to_page_cache()
Other than the unnecessary split line which Christoph pointed out, looks fine. You can add my Reviewed--by: Steve French On Tue, Oct 20, 2020 at 5:10 AM Christoph Hellwig wrote: > > > + rc = add_to_page_cache(page, mapping, > > +page->index, gfp); > > This trivially fits onto a single line. -- Thanks, Steve
Re: [PATCH] cifs: make const array static, makes object smaller
added Aurelien's Reviewed-by and merged into cifs-2.6.git for-next On Tue, Oct 20, 2020 at 9:22 AM Colin King wrote: > > From: Colin Ian King > > Don't populate const array smb3_create_tag_posix on the stack but > instead make it static. Makes the object code smaller by 50 bytes. > > Before: >textdata bss dec hex filename > 150184 47167 0 197351 302e7 fs/cifs/smb2pdu.o > > After: > text data bss dec hex filename > 150070 47231 0 197301 302b5 fs/cifs/smb2pdu.o > > (gcc version 10.2.0) > > Signed-off-by: Colin Ian King > --- > fs/cifs/smb2pdu.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index d504bc296349..be8696abd871 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -1971,9 +1971,11 @@ smb2_parse_contexts(struct TCP_Server_Info *server, > unsigned int next; > unsigned int remaining; > char *name; > - const char smb3_create_tag_posix[] = {0x93, 0xAD, 0x25, 0x50, 0x9C, > - 0xB4, 0x11, 0xE7, 0xB4, 0x23, 0x83, > - 0xDE, 0x96, 0x8B, 0xCD, 0x7C}; > + static const char smb3_create_tag_posix[] = { > + 0x93, 0xAD, 0x25, 0x50, 0x9C, > + 0xB4, 0x11, 0xE7, 0xB4, 0x23, 0x83, > + 0xDE, 0x96, 0x8B, 0xCD, 0x7C > + }; > > *oplock = 0; > data_offset = (char *)rsp + le32_to_cpu(rsp->CreateContextsOffset); > -- > 2.27.0 > -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
fixed On Tue, Sep 29, 2020 at 5:31 PM Stephen Rothwell wrote: > > Hi all, > > Commit > > 87505cefd88d ("Convert trailing spaces and periods in path components") > > is missing a Signed-off-by from its committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
[GIT PULL] SMB3 DFS Fix
Please pull the following changes since commit f4d51dffc6c01a9e94650d95ce0104964f8ae822: Linux 5.9-rc4 (2020-09-06 17:11:40 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.9-rc4-smb3-fix for you to fetch changes up to 01ec372cef1e5afa4ab843bbaf88a6fcb64dc14c: cifs: fix DFS mount with cifsacl/modefromsid (2020-09-06 23:59:53 -0500) A fix for lookup on DFS link when cifsacl or modefromsid used Build verification test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/397 Ronnie Sahlberg (1): cifs: fix DFS mount with cifsacl/modefromsid fs/cifs/inode.c | 4 1 file changed, 4 insertions(+) -- Thanks, Steve
[GIT PULL] CIFS Fix
Please pull the following changes since commit d012a7190fc1fd72ed48911e77ca97ba4521bccd: Linux 5.9-rc2 (2020-08-23 14:08:43 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.9-rc2-smb-fix for you to fetch changes up to e183785f2529b4135f00a0330a3b08e7c86530c8: cifs: fix check of tcon dfs in smb1 (2020-08-28 12:27:33 -0500) DFS fix for referral problem when using SMB1 Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/386 Paulo Alcantara (1): cifs: fix check of tcon dfs in smb1 fs/cifs/cifsglob.h | 15 +++ fs/cifs/connect.c | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) -- Thanks, Steve
[GIT PULL] SMB3 fixes
Please pull the following changes since commit 327a8d76b1ac2037f87bf041f3bc076407284ffc: Merge tag '5.9-rc-smb3-fixes-part1' of git://git.samba.org/sfrench/cifs-2.6 (2020-08-06 19:21:04 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.9-rc-smb3-fixes-part2 for you to fetch changes up to c8c412f976124d85b8ded85c6ac3f760c12b63a3: SMB3: Fix mkdir when idsfromsid configured on mount (2020-08-13 19:41:01 -0500) 3 small cifs/smb3 fixes, one for stable fixing a mkdir path when using idsfromsid mount option The update for cifs.ko to use the new mount API isn't complete so isn't included in this set. Build verification test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/381 Dan Carpenter (1): cifs: Fix an error pointer dereference in cifs_mount() Miaohe Lin (1): cifs: Convert to use the fallthrough macro Steve French (1): SMB3: Fix mkdir when idsfromsid configured on mount fs/cifs/connect.c | 1 + fs/cifs/smb2inode.c | 1 + fs/cifs/smb2pdu.c | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) -- Thanks, Steve
Re: [PATCH] cifs: Convert to use the fallthrough macro
merged into cifs-2.6.git for-next (but note that most places in fs directory other than cifs and btrfs have not been updated), and I noticed another 8 places in fs/cifs that you didn't change in your patch (ie change from the older way of indicating fallthrough /* Fallthrough */ as a comment to the newer fallthrough macro. On Sat, Aug 8, 2020 at 3:34 AM linmiaohe wrote: > > From: Miaohe Lin > > Convert the uses of fallthrough comments to fallthrough macro. > > Signed-off-by: Hongxiang Lou > Signed-off-by: Miaohe Lin > --- > fs/cifs/smb2pdu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 24c2ac360591..667d70aa335f 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -3913,7 +3913,7 @@ smb2_readv_callback(struct mid_q_entry *mid) > case MID_RESPONSE_MALFORMED: > credits.value = le16_to_cpu(shdr->CreditRequest); > credits.instance = server->reconnect_instance; > - /* fall through */ > + fallthrough; > default: > rdata->result = -EIO; > } > @@ -4146,7 +4146,7 @@ smb2_writev_callback(struct mid_q_entry *mid) > case MID_RESPONSE_MALFORMED: > credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); > credits.instance = server->reconnect_instance; > - /* fall through */ > + fallthrough; > default: > wdata->result = -EIO; > break; > -- > 2.19.1 > -- Thanks, Steve
Re: [GIT PULL] fscache rewrite -- please drop for now
On Mon, Aug 10, 2020 at 10:48 AM David Howells wrote: > > Steve French wrote: > > > cifs.ko also can set rsize quite small (even 1K for example, although > > that will be more than 10x slower than the default 4MB so hopefully no > > one is crazy enough to do that). > > You can set rsize < PAGE_SIZE? I have never seen anyone do it and it would be crazy to set it so small (would hurt performance a lot and cause extra work on client and server) but yes it can be set very small. Apparently NFS can also set rsize to 1K as well (see https://linux.die.net/man/5/nfs) I don't mind adding a minimum rsize check for cifs.ko (preventing a user from setting rsize below page size for example) if there is a precedent for this in other fs or bug that it would cause. In general my informal perf measurements showed slight advantages to all servers with larger rsizes up to 4MB (thus cifs client will negotiate 4MB by default even if server supports larger), but overriding rsize (larger) on mount by having the user setting rsize to 8MB on mount could help perf to some servers. I am hoping we can figure out a way to automatically determine when to negotiate rsize larger than 4MB but in the meantime rsize will almost always be 4MB (or 1MB on mounts to some older servers) for cifs but some users will benefit slightly from manually setting it to 8MB. > > I can't imagine an SMB3 server negotiating an rsize or wsize smaller than > > 64K in today's world (and typical is 1MB to 8MB) but the user can specify a > > much smaller rsize on mount. If 64K is an adequate minimum, we could change > > the cifs mount option parsing to require a certain minimum rsize if fscache > > is selected. > > I've borrowed the 256K granule size used by various AFS implementations for > the moment. A 512-byte xattr can thus hold a bitmap covering 1G of file > space. > > David > -- Thanks, Steve
Re: [GIT PULL] fscache rewrite -- please drop for now
cifs.ko also can set rsize quite small (even 1K for example, although that will be more than 10x slower than the default 4MB so hopefully no one is crazy enough to do that). I can't imagine an SMB3 server negotiating an rsize or wsize smaller than 64K in today's world (and typical is 1MB to 8MB) but the user can specify a much smaller rsize on mount. If 64K is an adequate minimum, we could change the cifs mount option parsing to require a certain minimum rsize if fscache is selected. On Mon, Aug 10, 2020 at 10:17 AM David Howells wrote: > > Hi Linus, > > Can you drop the fscache rewrite pull for now. We've seem an issue in NFS > integration and need to rework the read helper a bit. I made an assumption > that fscache will always be able to request that the netfs perform a read of a > certain minimum size - but with NFS you can break that by setting rsize too > small. > > We need to make the read helper able to make multiple netfs reads. This can > help ceph too. > > Thanks, > David > -- Thanks, Steve
Re: [PATCH] cifs: Convert to use the fallthrough macro
Is this conversion from "/* Fallthrough */" to the preferred (?) "fallthrough;" documented anywhere? All I see is a few fs changesets like: commit c730ae0c6bb3125ccb776fb2ab6abbdff500c02c Author: Marcos Paulo de Souza Date: Tue Jun 16 15:54:29 2020 -0300 btrfs: convert comments to fallthrough annotations Convert fall through comments to the pseudo-keyword which is now the preferred way. And the vast majority of places (33 vs. 4) use "/* Fallthrough */ in the fs directory On Sat, Aug 8, 2020 at 3:34 AM linmiaohe wrote: > > From: Miaohe Lin > > Convert the uses of fallthrough comments to fallthrough macro. > > Signed-off-by: Hongxiang Lou > Signed-off-by: Miaohe Lin > --- > fs/cifs/smb2pdu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 24c2ac360591..667d70aa335f 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -3913,7 +3913,7 @@ smb2_readv_callback(struct mid_q_entry *mid) > case MID_RESPONSE_MALFORMED: > credits.value = le16_to_cpu(shdr->CreditRequest); > credits.instance = server->reconnect_instance; > - /* fall through */ > + fallthrough; > default: > rdata->result = -EIO; > } > @@ -4146,7 +4146,7 @@ smb2_writev_callback(struct mid_q_entry *mid) > case MID_RESPONSE_MALFORMED: > credits.value = le16_to_cpu(rsp->sync_hdr.CreditRequest); > credits.instance = server->reconnect_instance; > - /* fall through */ > + fallthrough; > default: > wdata->result = -EIO; > break; > -- > 2.19.1 > -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commit in the cifs tree
Dan, I just fixed the Author tag in this patch to match your email address but seems like the author email address gets mangled when sent through some mailing lists. Any ideas how to avoid this. On Thu, Aug 6, 2020 at 1:45 AM Stephen Rothwell wrote: > > Hi all, > > Commit > > 2676d210d2f4 ("cifs: Fix an error pointer dereference in cifs_mount()") > > is missing a Signed-off-by from its author. > > This is only pisked up by my script because of the mangling of the email > sender address by the mailing list it passed through. I guess a little > more care is required to make sure the author attribution is correct in > this case. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit bcf876870b95592b52519ed4aafcf9d95999bc9c: Linux 5.8 (2020-08-02 14:21:45 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.9-rc-smb3-fixes-part1 for you to fetch changes up to 7efd081582619e7c270d1c0a422385dcaa99fa9f: cifs: document and cleanup dfs mount (2020-08-02 18:00:26 -0500) 16 cifs/smb3 fixes, about half DFS related, 2 fixes for stable Still working on and testing an additional set of fixes (including updates to mount, and some fallocate scenario improvements) for later in the merge window. Build verification test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/379 Colin Ian King (1): cifs: fix double free error on share and prefix Liao Pingfang (1): cifs: Remove the superfluous break Paul Aurich (1): cifs: Fix leak when handling lease break for cached root fid Paulo Alcantara (6): cifs: reduce number of referral requests in DFS link lookups cifs: rename reconn_inval_dfs_target() cifs: handle empty list of targets in cifs_reconnect() cifs: handle RESP_GET_DFS_REFERRAL.PathConsumed in reconnect cifs: only update prefix path of DFS links in cifs_tree_connect() cifs: document and cleanup dfs mount Qinglang Miao (1): cifs: convert to use be32_add_cpu() Randy Dunlap (1): cifs: delete duplicated words in header files Roberto Bergantinos Corpas (1): cifs`: handle ERRBaduid for SMB1 Ronnie Sahlberg (1): cifs: smb1: Try failing back to SetFileInfo if SetPathInfo fails Stefan Metzmacher (1): cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect() Steve French (1): smb3: warn on confusing error scenario with sec=krb5 Wei Yongjun (1): cifs: remove unused variable 'server' fs/cifs/cifsacl.h | 4 +- fs/cifs/cifsglob.h | 2 +- fs/cifs/cifsproto.h | 9 +- fs/cifs/cifssmb.c | 151 - fs/cifs/connect.c | 508 ++-- fs/cifs/dfs_cache.c | 136 +- fs/cifs/dfs_cache.h | 7 +- fs/cifs/inode.c | 2 - fs/cifs/misc.c | 7 +- fs/cifs/netmisc.c | 27 ++ fs/cifs/sess.c | 4 +- fs/cifs/smb1ops.c | 4 +- fs/cifs/smb2misc.c | 73 ++ fs/cifs/smb2pdu.c | 115 +- fs/cifs/smb2pdu.h | 2 +- fs/cifs/transport.c | 2 +- 16 files changed, 560 insertions(+), 493 deletions(-) -- Thanks, Steve
Re: [PATCH][next] cifs: fix double free error on share and prefix
merged into cifs-2.6.git for-next On Fri, Jul 31, 2020 at 12:15 PM Colin King wrote: > > From: Colin Ian King > > Currently if the call dfs_cache_get_tgt_share fails we cannot > fully guarantee that share and prefix are set to NULL and the > next iteration of the loop can end up potentially double freeing > these pointers. Since the semantics of dfs_cache_get_tgt_share > are ambiguous for failure cases with the setting of share and > prefix (currently now and the possibly the future), it seems > prudent to set the pointers to NULL when the objects are > free'd to avoid any double frees. > > Addresses-Coverity: ("Double free") > Fixes: 96296c946a2a ("cifs: handle RESP_GET_DFS_REFERRAL.PathConsumed in > reconnect") > Signed-off-by: Colin Ian King > --- > fs/cifs/connect.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 3c4dd4e1b9eb..4b2f5f5b3a8e 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -5574,6 +5574,8 @@ int cifs_tree_connect(const unsigned int xid, struct > cifs_tcon *tcon, const stru > > kfree(share); > kfree(prefix); > + share = NULL; > + prefix = NULL; > > rc = dfs_cache_get_tgt_share(tcon->dfs_path + 1, it, , > ); > if (rc) { > -- > 2.27.0 > -- Thanks, Steve
Re: [PATCH -next] cifs: convert to use be32_add_cpu()
merged into cifs-2.6.git for-next and for-next also updated with 5.8-rc7 On Sat, Jul 25, 2020 at 3:53 AM Qinglang Miao wrote: > > Convert cpu_to_be32(be32_to_cpu(E1) + E2) to use be32_add_cpu(). > > Signed-off-by: Qinglang Miao > --- > fs/cifs/connect.c | 3 +-- > fs/cifs/sess.c| 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 889fee586..552975420 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -5114,8 +5114,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses, > bcc_ptr += strlen("?"); > bcc_ptr += 1; > count = bcc_ptr - >Password[0]; > - pSMB->hdr.smb_buf_length = cpu_to_be32(be32_to_cpu( > - pSMB->hdr.smb_buf_length) + count); > + be32_add_cpu(>hdr.smb_buf_length, count); > pSMB->ByteCount = cpu_to_le16(count); > > rc = SendReceive(xid, ses, smb_buffer, smb_buffer_response, , > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index 6708ab0aa..69cd58566 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -938,8 +938,7 @@ sess_sendreceive(struct sess_data *sess_data) > struct kvec rsp_iov = { NULL, 0 }; > > count = sess_data->iov[1].iov_len + sess_data->iov[2].iov_len; > - smb_buf->smb_buf_length = > - cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count); > + be32_add_cpu(_buf->smb_buf_length, count); > put_bcc(count, smb_buf); > > rc = SendReceive2(sess_data->xid, sess_data->ses, > -- > 2.25.1 > -- Thanks, Steve
Re: [PATCH] cifs: delete duplicated words in header files
Merged into cifs-2.6.git for-next On Sun, Jul 19, 2020 at 7:14 PM Randy Dunlap wrote: > > Drop repeated words in multiple comments. > (be, use, the, See) > > Signed-off-by: Randy Dunlap > Cc: Steve French > Cc: linux-c...@vger.kernel.org > Cc: samba-techni...@lists.samba.org > --- > fs/cifs/cifsacl.h |4 ++-- > fs/cifs/cifsglob.h |2 +- > fs/cifs/smb2pdu.h |2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > --- linux-next-20200717.orig/fs/cifs/cifsacl.h > +++ linux-next-20200717/fs/cifs/cifsacl.h > @@ -132,7 +132,7 @@ struct cifs_ace { > /* > * The current SMB3 form of security descriptor is similar to what was used > for > * cifs (see above) but some fields are split, and fields in the struct below > - * matches names of fields to the the spec, MS-DTYP (see sections 2.4.5 and > + * matches names of fields to the spec, MS-DTYP (see sections 2.4.5 and > * 2.4.6). Note that "CamelCase" fields are used in this struct in order to > * match the MS-DTYP and MS-SMB2 specs which define the wire format. > */ > @@ -178,7 +178,7 @@ struct smb3_acl { > > /* > * Used to store the special 'NFS SIDs' used to persist the POSIX uid and gid > - * See See http://technet.microsoft.com/en-us/library/hh509017(v=ws.10).aspx > + * See http://technet.microsoft.com/en-us/library/hh509017(v=ws.10).aspx > */ > struct owner_sid { > u8 Revision; > --- linux-next-20200717.orig/fs/cifs/cifsglob.h > +++ linux-next-20200717/fs/cifs/cifsglob.h > @@ -1466,7 +1466,7 @@ struct cifsInodeInfo { > struct list_head llist; /* locks helb by this inode */ > /* > * NOTE: Some code paths call down_read(lock_sem) twice, so > -* we must always use use cifs_down_write() instead of down_write() > +* we must always use cifs_down_write() instead of down_write() > * for this semaphore to avoid deadlocks. > */ > struct rw_semaphore lock_sem; /* protect the fields above */ > --- linux-next-20200717.orig/fs/cifs/smb2pdu.h > +++ linux-next-20200717/fs/cifs/smb2pdu.h > @@ -31,7 +31,7 @@ > * Note that, due to trying to use names similar to the protocol > specifications, > * there are many mixed case field names in the structures below. Although > * this does not match typical Linux kernel style, it is necessary to be > - * be able to match against the protocol specfication. > + * able to match against the protocol specfication. > * > * SMB2 commands > * Some commands have minimal (wct=0,bcc=0), or uninteresting, responses -- Thanks, Steve
Re: 5.8-rc1 and later breaks chrome browser
This has been fixed in 5.8-rc5. Chrome now works again (failed on rc1, rc2 and rc4, worked on 5.7 and before as well) On Sat, Jul 4, 2020 at 3:29 PM Steve French wrote: > > I noticed that chrome crashes immediately on startup on my desktop > booting to 5.8-rc2 or later kernels (whether I build the kernel or > using the prebuilt weekly Ubuntu mainline kernel downloads). Works > fine with default kernels or 5.7 or 5.7.7 stable kernel etc. - just > breaks if I boot 5.8-rc2 or later. I even tried building with today's > mainline kernel and same thing. Any ideas how to work around this. > > Message logged to dmesg is: > > [ 131.366543] ThreadPoolServi[2526]: segfault at 415048 ip > 5652def3370d sp 7fcdc3b37df0 error 6 in > chrome[5652daa7e000+785b000] > [ 131.366546] Code: Bad RIP value. > > -- > Thanks, > > Steve -- Thanks, Steve
Re: [PATCH] cifs: Remove the superfluous break
merged into cifs-2.6.git for-next thx On Tue, Jul 14, 2020 at 6:14 AM Yi Wang wrote: > > From: Liao Pingfang > > Remove the superfuous break, as there is a 'return' before it. > > Signed-off-by: Liao Pingfang > Signed-off-by: Yi Wang > --- > fs/cifs/sess.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index 5d05bd2..6708ab0 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -1705,7 +1705,6 @@ static int select_sec(struct cifs_ses *ses, struct > sess_data *sess_data) > #else > cifs_dbg(VFS, "Kerberos negotiated but upcall support > disabled!\n"); > return -ENOSYS; > - break; > #endif /* CONFIG_CIFS_UPCALL */ > case RawNTLMSSP: > sess_data->func = sess_auth_rawntlmssp_negotiate; > -- > 2.9.5 > -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit dcb7fd82c75ee2d6e6f9d8cc71c52519ed52e258: Linux 5.8-rc4 (2020-07-05 16:20:22 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.8-rc4-smb3-fixes for you to fetch changes up to a8dab63ea623610bb258d93649e30330dd1b7c8b: cifs: update internal module version number (2020-07-09 10:07:09 -0500) 4 cifs/smb3 fixes: the three for stable fix problems found recently with change notification including a reference count leak Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/367 Ronnie Sahlberg (1): cifs: fix reference leak for tlink Steve French (3): smb3: fix access denied on change notify request to some servers smb3: fix unneeded error message on change notify cifs: update internal module version number yangerkun (1): cifs: remove the retry in cifs_poxis_lock_set fs/cifs/cifsfs.h | 2 +- fs/cifs/file.c | 19 ++- fs/cifs/ioctl.c| 9 - fs/cifs/smb2misc.c | 8 ++-- fs/cifs/smb2ops.c | 2 +- 5 files changed, 22 insertions(+), 18 deletions(-) -- Thanks, Steve
5.8-rc1 and later breaks chrome browser
I noticed that chrome crashes immediately on startup on my desktop booting to 5.8-rc2 or later kernels (whether I build the kernel or using the prebuilt weekly Ubuntu mainline kernel downloads). Works fine with default kernels or 5.7 or 5.7.7 stable kernel etc. - just breaks if I boot 5.8-rc2 or later. I even tried building with today's mainline kernel and same thing. Any ideas how to work around this. Message logged to dmesg is: [ 131.366543] ThreadPoolServi[2526]: segfault at 415048 ip 5652def3370d sp 7fcdc3b37df0 error 6 in chrome[5652daa7e000+785b000] [ 131.366546] Code: Bad RIP value. -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68: Linux 5.8-rc3 (2020-06-28 15:00:24 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.8-rc3-smb3-fixes for you to fetch changes up to 19e888678bac8c82206eb915eaf72741b2a2615c: cifs: prevent truncation from long to int in wait_for_free_credits (2020-07-01 20:01:26 -0500) 8 cifs/smb3 fixes, most for when specifying the multiuser mount flag, 5 of the fixes for stable. Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/364 Paul Aurich (6): cifs: Display local UID details for SMB sessions in DebugData SMB3: Honor 'seal' flag for multiuser mounts SMB3: Honor persistent/resilient handle flags for multiuser mounts SMB3: Honor lease disabling for multiuser mounts SMB3: Honor 'handletimeout' flag for multiuser mounts SMB3: Honor 'posix' flag for multiuser mounts Ronnie Sahlberg (1): cifs: prevent truncation from long to int in wait_for_free_credits Zhang Xiaoxu (1): cifs: Fix the target file was deleted when rename failed. fs/cifs/cifs_debug.c | 6 +- fs/cifs/connect.c| 10 ++ fs/cifs/inode.c | 10 -- fs/cifs/transport.c | 2 +- 4 files changed, 20 insertions(+), 8 deletions(-) -- Thanks, Steve
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit 48778464bb7d346b47157d21ffde2af6b2d39110: Linux 5.8-rc2 (2020-06-21 15:45:29 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.8-rc2-smb3-fixes for you to fetch changes up to bf1028a41eaf0ce39518cbdda34cdb717f16364a: cifs: misc: Use array_size() in if-statement controlling expression (2020-06-23 19:06:27 -0500) 6 cifs/smb3 fixes, 3 for stable. Fixes xfstests 451, 313 and 316 Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/360 Gustavo A. R. Silva (1): cifs: misc: Use array_size() in if-statement controlling expression Xiyu Yang (1): cifs: Fix cached_fid refcnt leak in open_shroot Zhang Xiaoxu (4): cifs: Fix double add page to memcg when cifs_readpages cifs/smb3: Fix data inconsistent when zero file range cifs/smb3: Fix data inconsistent when punch hole cifs: update ctime and mtime during truncate fs/cifs/file.c| 11 +++ fs/cifs/inode.c | 9 + fs/cifs/misc.c| 16 +++- fs/cifs/smb2ops.c | 12 4 files changed, 35 insertions(+), 13 deletions(-) -- Thanks, Steve
Re: linux-next: Signed-off-by missing for commits in the cifs tree
fixed cifs-2.6.git for-next updated thx On Tue, Jun 23, 2020 at 5:31 PM Stephen Rothwell wrote: > > Hi all, > > Commits > > 52e2b5b30cee ("cifs/smb3: Fix data inconsistent when punch hole") > 76f77967b39e ("cifs/smb3: Fix data inconsistent when zero file range") > > are missing a Signed-off-by from their committer. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: [PATCH] cifs: misc: Use array_size() in if-statement controlling expression
Added the two reviewed-bys and merged into cifs-2.6.git for-next On Tue, Jun 16, 2020 at 6:17 AM Aurélien Aptel via samba-technical wrote: > > Reviewed-by: Aurelien Aptel > > Cheers, > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) > -- Thanks, Steve
Re: [PATCH] cifs: Fix cached_fid refcnt leak in open_shroot
added cc:stable and merged into cifs-2.6.git for-next On Sat, Jun 13, 2020 at 7:36 AM Xiyu Yang wrote: > > open_shroot() invokes kref_get(), which increases the refcount of the > "tcon->crfid" object. When open_shroot() returns not zero, it means the > open operation failed and close_shroot() will not be called to decrement > the refcount of the "tcon->crfid". > > The reference counting issue happens in one normal path of > open_shroot(). When the cached root have been opened successfully in a > concurrent process, the function increases the refcount and jump to > "oshr_free" to return. However the current return value "rc" may not > equal to 0, thus the increased refcount will not be balanced outside the > function, causing a refcnt leak. > > Fix this issue by setting the value of "rc" to 0 before jumping to > "oshr_free" label. > > Signed-off-by: Xiyu Yang > Signed-off-by: Xin Tan > --- > fs/cifs/smb2ops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 736d86b8a910..28553d45604e 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -763,6 +763,7 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, > /* close extra handle outside of crit sec */ > SMB2_close(xid, tcon, fid.persistent_fid, > fid.volatile_fid); > } > + rc = 0; > goto oshr_free; > } > > -- > 2.7.4 > -- Thanks, Steve
[GIT PULL] CIFS/SMB3 Fixes
Please pull the following changes since commit 3803d5e4d3ce2600ffddc16a1999798bc719042d: Merge tag '5.8-rc-smb3-fixes-part-1' of git://git.samba.org/sfrench/cifs-2.6 (2020-06-05 16:40:53 -0700) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.8-rc-smb3-fixes-part2 for you to fetch changes up to a7a519a4926214ba4161bc30109f4a8d69defb8d: smb3: Add debug message for new file creation with idsfromsid mount option (2020-06-12 16:31:06 -0500) 12 cifs/smb3 fixes, 2 for stable. Adds support for idsfromsid on create and also for chgrp/chown allowing ability to save owner information more naturally for some workloads. Improves getattr when SMB3.1.1 posix extensions are negotiated by using new query info level. Regression test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/357 Kenneth D'souza (1): cifs: Add get_security_type_str function to return sec type. Namjae Jeon (1): smb3: add indatalen that can be a non-zero value to calculation of credit charge in smb2 ioctl Steve French (10): smb3: extend fscache mount volume coherency check smb3: fix typo in mount options displayed in /proc/mounts SMB311: Add support for query info using posix extensions (level 100) smb311: Add support for SMB311 query info (non-compounded) smb311: Add support for lookup with posix extensions query info smb311: add support for using info level for posix extensions query smb311: Add tracepoints for new compound posix query info smb3: allow uid and gid owners to be set on create with idsfromsid mount option cifs: fix chown and chgrp when idsfromsid mount option enabled smb3: Add debug message for new file creation with idsfromsid mount option fs/cifs/cache.c | 9 +--- fs/cifs/cifs_debug.c | 4 +- fs/cifs/cifsacl.c| 79 -- fs/cifs/cifsacl.h| 15 ++ fs/cifs/cifsfs.c | 2 +- fs/cifs/cifsglob.h | 18 +++ fs/cifs/cifsproto.h | 3 ++ fs/cifs/dir.c| 5 +- fs/cifs/file.c | 5 +- fs/cifs/fscache.c| 17 ++- fs/cifs/fscache.h| 9 fs/cifs/inode.c | 185 -- fs/cifs/link.c | 4 +- fs/cifs/smb2glob.h | 1 + fs/cifs/smb2inode.c | 105 fs/cifs/smb2pdu.c| 131 - fs/cifs/smb2pdu.h| 27 ++- fs/cifs/smb2proto.h | 6 +++ fs/cifs/trace.h | 3 ++ 19 files changed, 571 insertions(+), 57 deletions(-) -- Thanks, Steve
Re: [PATCH] cifs: Standardize logging output
tentatively merged into cifs-2.6.git for-next but had to clean it up to avoid merge conflicts. Minor followon patch (attached) to add the two remaining ones that Joe pointed out that. On Wed, Apr 15, 2020 at 12:46 AM Joe Perches via samba-technical wrote: > > Use pr_fmt to standardize all logging for fs/cifs. > > Some logging output had no CIFS: specific prefix. > > Now all output has one of three prefixes: > > o CIFS: > o CIFS: VFS: > o Root-CIFS: > > Miscellanea: > > o Convert printks to pr_ > o Neaten macro definitions > o Remove embedded CIFS: prefixes from formats > o Convert "illegal" to "invalid" > o Coalesce formats > o Add missing '\n' format terminations > o Consolidate multiple cifs_dbg continuations into single calls > o More consistent use of upper case first word output logging > o Multiline statement argument alignment and wrapping > > Signed-off-by: Joe Perches > --- > fs/cifs/cifs_debug.h | 145 ++--- > fs/cifs/cifsencrypt.c | 8 +- > fs/cifs/cifsproto.h | 26 +++ > fs/cifs/cifsroot.c| 6 +- > fs/cifs/cifssmb.c | 26 +++ > fs/cifs/connect.c | 77 > fs/cifs/dfs_cache.c | 14 ++-- > fs/cifs/file.c| 24 +++--- > fs/cifs/inode.c | 4 +- > fs/cifs/misc.c| 12 +-- > fs/cifs/netmisc.c | 6 +- > fs/cifs/readdir.c | 10 +-- > fs/cifs/sess.c| 28 +++ > fs/cifs/smb1ops.c | 2 +- > fs/cifs/smb2inode.c | 3 +- > fs/cifs/smb2misc.c| 20 ++--- > fs/cifs/smb2ops.c | 31 > fs/cifs/smb2pdu.c | 72 +- > fs/cifs/smbdirect.c | 165 +- > fs/cifs/transport.c | 25 +++ > 20 files changed, 321 insertions(+), 383 deletions(-) > > diff --git a/fs/cifs/cifs_debug.h b/fs/cifs/cifs_debug.h > index 100b00..5e66da 100644 > --- a/fs/cifs/cifs_debug.h > +++ b/fs/cifs/cifs_debug.h > @@ -8,6 +8,12 @@ > #ifndef _H_CIFS_DEBUG > #define _H_CIFS_DEBUG > > +#ifdef pr_fmt > +#undef pr_fmt > +#endif > + > +#define pr_fmt(fmt) "CIFS: " fmt > + > void cifs_dump_mem(char *label, void *data, int length); > void cifs_dump_detail(void *buf, struct TCP_Server_Info *ptcp_info); > void cifs_dump_mids(struct TCP_Server_Info *); > @@ -46,92 +52,81 @@ extern int cifsFYI; > */ > > /* Information level messages, minor events */ > -#define cifs_info_func(ratefunc, fmt, ...) \ > -do { \ > - pr_info_ ## ratefunc("CIFS: " fmt, ##__VA_ARGS__); \ > -} while (0) > +#define cifs_info_func(ratefunc, fmt, ...) \ > + pr_info_ ## ratefunc(fmt, ##__VA_ARGS__) > > -#define cifs_info(fmt, ...)\ > -do { \ > - cifs_info_func(ratelimited, fmt, ##__VA_ARGS__);\ > -} while (0) > +#define cifs_info(fmt, ...)\ > + cifs_info_func(ratelimited, fmt, ##__VA_ARGS__) > > /* information message: e.g., configuration, major event */ > -#define cifs_dbg_func(ratefunc, type, fmt, ...)\ > -do { \ > - if ((type) & FYI && cifsFYI & CIFS_INFO) { \ > - pr_debug_ ## ratefunc("%s: "\ > - fmt, __FILE__, ##__VA_ARGS__); \ > - } else if ((type) & VFS) { \ > - pr_err_ ## ratefunc("CIFS VFS: "\ > -fmt, ##__VA_ARGS__); \ > - } else if ((type) & NOISY && (NOISY != 0)) {\ > - pr_debug_ ## ratefunc(fmt, ##__VA_ARGS__); \ > - } \ > +#define cifs_dbg_func(ratefunc, type, fmt, ...) > \ > +do { \ > + if ((type) & FYI && cifsFYI & CIFS_INFO) { \ > + pr_debug_ ## ratefunc("%s: " fmt, \ > + __FILE__, ##__VA_ARGS__); \ > + } else if ((type) & VFS) { \ > + pr_err_ ## ratefunc("VFS: " fmt, ##__VA_ARGS__);\ > + } else if ((type) & NOISY && (NOISY != 0)) {\ > + pr_debug_ ## ratefunc(fmt, ##__VA_ARGS__); \ > + } \ > } while (0) > > -#define cifs_dbg(type, fmt, ...) \ > -do { \ > - if ((type) & ONCE) \ > - cifs_dbg_func(once, \ > -type, fmt, ##__VA_ARGS__); \ > - else
Re: [PATCH] cifs: remove redundant initialization of variable rc
merged into cifs-2.6.git for-next On Wed, May 27, 2020 at 7:52 AM Colin King wrote: > > From: Colin Ian King > > The variable rc is being initialized with a value that is never read > and it is being updated later with a new value. The initialization is > redundant and can be removed. > > Addresses-Coverity: ("Unused value") > Signed-off-by: Colin Ian King > --- > fs/cifs/cifssmb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 5014a82391ff..d62f9175c546 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -2375,7 +2375,7 @@ int > CIFSSMBWrite2(const unsigned int xid, struct cifs_io_parms *io_parms, > unsigned int *nbytes, struct kvec *iov, int n_vec) > { > - int rc = -EACCES; > + int rc; > WRITE_REQ *pSMB = NULL; > int wct; > int smb_hdr_len; > -- > 2.25.1 > -- Thanks, Steve
Re: linux-next: build warning after merge of the tip tree
Ronnie mentioned that he will take a look ... On Tue, May 19, 2020 at 1:57 AM Stephen Rothwell wrote: > > Hi all, > > After merging the tip tree, today's linux-next build (x86_64 allmodconfig) > produced this warning: > > fs/cifs/smb2inode.c: In function 'smb2_compound_op': > fs/cifs/smb2inode.c:424:1: warning: the frame size of 2736 bytes is larger > than 2048 bytes [-Wframe-larger-than=] > 424 | } > | ^ > > I have no idea what caused this. > > -- > Cheers, > Stephen Rothwell -- Thanks, Steve
Re: [PATCH trivial] CIFS: Spelling s/EACCESS/EACCES/
merged into cifs-2.6.git for-next On Tue, May 5, 2020 at 8:49 AM Geert Uytterhoeven wrote: > > As per POSIX, the correct spelling is EACCES: > > include/uapi/asm-generic/errno-base.h:#define EACCES 13 /* Permission denied > */ > > Fixes: b8f7442bc46e48fb ("CIFS: refactor cifs_get_inode_info()") > Signed-off-by: Geert Uytterhoeven > --- > fs/cifs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 390d2b15ef6ef9d7..5d2965a2373054a4 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -730,7 +730,7 @@ static __u64 simple_hashstr(const char *str) > * cifs_backup_query_path_info - SMB1 fallback code to get ino > * > * Fallback code to get file metadata when we don't have access to > - * @full_path (EACCESS) and have backup creds. > + * @full_path (EACCES) and have backup creds. > * > * @data will be set to search info result buffer > * @resp_buf will be set to cifs resp buf and needs to be freed with > -- > 2.17.1 > -- Thanks, Steve
Re: [PATCH -next] CIFS: remove set but not used variables 'cinode' and 'netfid'
tentatively pushed to cifs-2.6.git for-next pending more testing of the flock patch it modified. On Fri, Oct 18, 2019 at 1:07 AM YueHaibing wrote: > > Fixes gcc '-Wunused-but-set-variable' warning: > > fs/cifs/file.c: In function 'cifs_flock': > fs/cifs/file.c:1704:8: warning: > variable 'netfid' set but not used [-Wunused-but-set-variable] > > fs/cifs/file.c:1702:24: warning: > variable 'cinode' set but not used [-Wunused-but-set-variable] > > Reported-by: Hulk Robot > Signed-off-by: YueHaibing > --- > fs/cifs/file.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 936e03892e2a..02a81dc6861a 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -1699,9 +1699,7 @@ int cifs_flock(struct file *file, int cmd, struct > file_lock *fl) > bool posix_lck = false; > struct cifs_sb_info *cifs_sb; > struct cifs_tcon *tcon; > - struct cifsInodeInfo *cinode; > struct cifsFileInfo *cfile; > - __u16 netfid; > __u32 type; > > rc = -EACCES; > @@ -1716,8 +1714,6 @@ int cifs_flock(struct file *file, int cmd, struct > file_lock *fl) > cifs_read_flock(fl, , , , _flag, > tcon->ses->server); > cifs_sb = CIFS_FILE_SB(file); > - netfid = cfile->fid.netfid; > - cinode = CIFS_I(file_inode(file)); > > if (cap_unix(tcon->ses) && > (CIFS_UNIX_FCNTL_CAP & le64_to_cpu(tcon->fsUnixInfo.Capability)) > && > > > -- Thanks, Steve
Re: [PATCH] cifs: Fix missed free operations
Merged into cifs-2.6.git for-next and cc: stable for 5.3 On Mon, Oct 14, 2019 at 4:43 PM Pavel Shilovsky via samba-technical wrote: > > пн, 14 окт. 2019 г. в 00:18, Chuhong Yuan : > > > > cifs_setattr_nounix has two paths which miss free operations > > for xid and fullpath. > > Use goto cifs_setattr_exit like other paths to fix them. > > > > Fixes: aa081859b10c ("cifs: flush before set-info if we have writeable > > handles") > > Signed-off-by: Chuhong Yuan > > --- > > fs/cifs/inode.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > > index 5dcc95b38310..df9377828e2f 100644 > > --- a/fs/cifs/inode.c > > +++ b/fs/cifs/inode.c > > @@ -2475,9 +2475,9 @@ cifs_setattr_nounix(struct dentry *direntry, struct > > iattr *attrs) > > rc = tcon->ses->server->ops->flush(xid, tcon, > > >fid); > > cifsFileInfo_put(wfile); > > if (rc) > > - return rc; > > + goto cifs_setattr_exit; > > } else if (rc != -EBADF) > > - return rc; > > + goto cifs_setattr_exit; > > else > > rc = 0; > > } > > -- > > 2.20.1 > > > > Looks good, thanks. > > Reviewed-by: Pavel Shilovsky > > The original patch was tagged for stable, so, it seems that this one > should be tagged too. > > -- > Best regards, > Pavel Shilovsky > -- Thanks, Steve
[no subject]
I noticed that the commit below regressed cifs/smb3 xfstest 258 on 5.4-rc1 and later. "Testing for negative seconds since epoch" "Timestamp wrapped" Did xfstest 258 get updated to account for the new behavior with this patch? commit cb7a69e605908c34aad47644afeb26a765ade8d7 Author: Deepa Dinamani Date: Fri Mar 22 14:32:35 2019 -0700 fs: cifs: Initialize filesystem timestamp ranges Fill in the appropriate limits to avoid inconsistencies in the vfs cached inode times when timestamps are outside the permitted range. Also fixed cnvrtDosUnixTm calculations to avoid int overflow while computing maximum date. -- Thanks, Steve
Re: [cifs] semantics of IPC$ shares (was Re: [PATCH] devpts: Fix NULL pointer dereference in dcache_readdir())
Your questions are interesting and rarely asked. On Fri, Oct 4, 2019 at 11:57 AM Al Viro wrote: > > On Fri, Oct 04, 2019 at 05:02:20PM +0100, Al Viro wrote: > > > * (possibly) cifs hitting the same on eviction by memory pressure > > alone > > (no locked inodes anywhere in sight). Possibly == if cifs IPC$ share > > happens to > > show up non-empty (e.g. due to server playing silly buggers). > > * (possibly) cifs hitting *another* lovely issue - lookup in one > > subdirectory > > of IPC$ root finding an alias for another subdirectory of said root, > > triggering > > d_move() of dentry of the latter. IF the name happens to be long enough to > > be > > externally allocated and if dcache_readdir() on root is currently copying > > it to > > userland, Bad Things(tm) will happen. That one almost certainly depends > > upon the > > server playing silly buggers and might or might not be possible. I'm not > > familiar > > enough with CIFS to tell. > > BTW, I would really appreciate somebody familiar with CIFS giving a braindump > on > that. Questions: > > 1) What's normally (== without malicious/broken server) seen when you mount > an IPC$ share? IPC$ is for "inter process communication" so is basically an abstraction for named pipes (used for remote procedure call queries using the old DCE/RPC standard). To Windows it is possible to mount IPC$, to Samba you can connect to the share but due to a Samba server bug you can't do a query info on "." (the 'root' of the IPC$ share). > 2) Does it ever have subdirectories (i.e. can we fail a lookup in its root if > it > looks like returning a subdirectory)? In Samba you can't query subdirectories on IPC$ because even open of "." fails, but to Windows the query directory would get "STATUS_INVALID_INFO_CLASS" An interesting question, and one that I will bring up with the spec writers is whether there are info level which would be allowed for query directory (probably not). Another interesting question this brings up is ... "should we allow enumerating the 'services' under IPC$ via readdir"? You could imagine a case where mounting IPC$ would allow you to see the 'services' exported by the server over remote procedure call ("server service" and "workstation server" and "netlogon service" and the global name space (DFS) service and perhaps "witness protocol services" and "branch cache service" etc.) And then thinking about Dave Howell's changes to the mount API - should this be a mechanism that is allowed to be used to either browse the valid shares or better access the root of the (DFS) global name space. But the short answer is "no you can't query the directory contents under IPC$" (at least not without changing the abstraction that we export on the client) but I am open to ideas if this would fit with Dave Howell's changes to the mount API or other ideas. > 3) If it can be non-empty, is there way to ask the server about its contents? > Short of "look every possible name up", that is... > > As it is, the thing is abusing either cifs_lookup() (if it really shouldn't > have any files in it) or dcache_readdir(). Sure, dcache_readdir() can (and > should) pin a dentry while copying the name to userland, but WTF kind of > semantics it is? "ls will return the things you'd guessed to look up, until > there's enough memory pressure to have them forgotten, which can happen at > any point with no activity on server"? Server's realistically must expose a share "IPC$" so in theory it can be mounted (despite Samba server's current bug there) and there were some experiments that Shirish did a few years ago opening well known services under mounts to IPC$ (to allow doing remote procedure calls over SMB3 mounts which has some value) but AFAIK you would never do a readdir over IPC$ and no current users would ever mount IPC$ -- Thanks, Steve
Re: nsdeps not working on modules in 5.4-rc1
ok - so to sum up, it sounds like you are saying the 350 false positives (see attached file) that happen on building cifs.ko .will be fixed by a future change to modpost? This is a typical module build ... download and install current kernel package (in this case Ubuntu 5.4-rc1). which saves huge amount of build time, then build just the module of interest (in my case cifs.ko) cd fs/cifs make C=1 -C /usr/src/linux-headers-`uname -r` M=`pwd` modules If nsdeps is not needed to fixup some namespace issue then shouldn't be a problem, just trying to avoid the distraction of 300+ warning messages every time I build just this one module. Is there a workaround? On Thu, Oct 3, 2019 at 11:51 PM Masahiro Yamada wrote: > > Hi Steve, > > On Fri, Oct 4, 2019 at 1:28 PM Steve French wrote: > > > > On Thu, Oct 3, 2019 at 10:41 PM Masahiro Yamada > > wrote: > > > > > > Hi Steve, > > > > > > On Fri, Oct 4, 2019 at 1:07 AM Steve French wrote: > > > > > > > > On Thu, Oct 3, 2019 at 10:24 AM Masahiro Yamada > > > > wrote: > > > > > > > > > > Hi Steve, > > > > > > > > > > On Fri, Oct 4, 2019 at 12:15 AM Steve French > > > > > wrote: > > > > > > > > > > > > On Thu, Oct 3, 2019 at 5:43 AM Matthias Maennich > > > > > > wrote: > > > > > > > > > > > > > > Hi Steve! > > > > > > > > > > > > > > On Wed, Oct 02, 2019 at 06:54:26PM -0500, Steve French wrote: > > > > > > > >And running the build differently, from the root of the git tree > > > > > > > >(5.4-rc1) rather than using the Ubuntu 5.4-rc1 headers also fails > > > > > > > > > > > > > > > >e.g. "make M=fs/cifs modules nsdeps" > > > > > > > > > > > > > > > >... > > > > > > > > LD [M] fs/cifs/cifs.o > > > > > > > > Building modules, stage 2. > > > > > > > > MODPOST 1 modules > > > > > > > >WARNING: module cifs uses symbol sigprocmask from namespace > > > > > > > >_fs/cifs/cache.o), but does not import it. > > > > > > > >... > > > > > > > >WARNING: module cifs uses symbol posix_test_lock from namespace > > > > > > > >cifs/cache.o), but does not import it. > > > > > > > > CC [M] fs/cifs/cifs.mod.o > > > > > > > > LD [M] fs/cifs/cifs.ko > > > > > > > > Building modules, stage 2. > > > > > > > > MODPOST 1 modules > > > > > > > >./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable > > > > > > > >name > > > > > > > >make: *** [Makefile:1710: nsdeps] Error 2 > > > > > > > > > > > > > > Thanks for reporting this. It appears to me you hit a bug that was > > > > > > > recently discovered: when building with `make > > > > > > > M=some/subdirectory`, > > > > > > > modpost is misbehaving. Can you try whether this patch series > > > > > > > solves > > > > > > > your problems: > > > > > > > https://lore.kernel.org/lkml/20191003075826.7478-1-yamada.masah...@socionext.com/ > > > > > > > In particular patch 2/6 out of the series. > > > > > > > > > > > > > > Cheers, > > > > > > > Matthias > > > > > > > > > > > > > > > > > > Applying just patch 2 and doing "make" from the root of the git tree > > > > > > (5.4-rc1), at the tail end of the build I got > > > > > > > > > > > > ... > > > > > > Kernel: arch/x86/boot/bzImage is ready (#87) > > > > > > Building modules, stage 2. > > > > > > MODPOST 5340 modules > > > > > > free(): invalid pointer > > > > > > Aborted (core dumped) > > > > > > > > > > > > > > > Right. > > > > > > > > > > Since 2/6 depends on 1/6, > > > > > applying only the second one does not work. > > > > > > > > Applying both 1 and 2 I get the following error doing make (although > > > > it makes it a long way into
Re: nsdeps not working on modules in 5.4-rc1
On Thu, Oct 3, 2019 at 10:41 PM Masahiro Yamada wrote: > > Hi Steve, > > On Fri, Oct 4, 2019 at 1:07 AM Steve French wrote: > > > > On Thu, Oct 3, 2019 at 10:24 AM Masahiro Yamada > > wrote: > > > > > > Hi Steve, > > > > > > On Fri, Oct 4, 2019 at 12:15 AM Steve French wrote: > > > > > > > > On Thu, Oct 3, 2019 at 5:43 AM Matthias Maennich > > > > wrote: > > > > > > > > > > Hi Steve! > > > > > > > > > > On Wed, Oct 02, 2019 at 06:54:26PM -0500, Steve French wrote: > > > > > >And running the build differently, from the root of the git tree > > > > > >(5.4-rc1) rather than using the Ubuntu 5.4-rc1 headers also fails > > > > > > > > > > > >e.g. "make M=fs/cifs modules nsdeps" > > > > > > > > > > > >... > > > > > > LD [M] fs/cifs/cifs.o > > > > > > Building modules, stage 2. > > > > > > MODPOST 1 modules > > > > > >WARNING: module cifs uses symbol sigprocmask from namespace > > > > > >_fs/cifs/cache.o), but does not import it. > > > > > >... > > > > > >WARNING: module cifs uses symbol posix_test_lock from namespace > > > > > >cifs/cache.o), but does not import it. > > > > > > CC [M] fs/cifs/cifs.mod.o > > > > > > LD [M] fs/cifs/cifs.ko > > > > > > Building modules, stage 2. > > > > > > MODPOST 1 modules > > > > > >./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name > > > > > >make: *** [Makefile:1710: nsdeps] Error 2 > > > > > > > > > > Thanks for reporting this. It appears to me you hit a bug that was > > > > > recently discovered: when building with `make M=some/subdirectory`, > > > > > modpost is misbehaving. Can you try whether this patch series solves > > > > > your problems: > > > > > https://lore.kernel.org/lkml/20191003075826.7478-1-yamada.masah...@socionext.com/ > > > > > In particular patch 2/6 out of the series. > > > > > > > > > > Cheers, > > > > > Matthias > > > > > > > > > > > > Applying just patch 2 and doing "make" from the root of the git tree > > > > (5.4-rc1), at the tail end of the build I got > > > > > > > > ... > > > > Kernel: arch/x86/boot/bzImage is ready (#87) > > > > Building modules, stage 2. > > > > MODPOST 5340 modules > > > > free(): invalid pointer > > > > Aborted (core dumped) > > > > > > > > > Right. > > > > > > Since 2/6 depends on 1/6, > > > applying only the second one does not work. > > > > Applying both 1 and 2 I get the following error doing make (although > > it makes it a long way into the build) > > > > > > WARNING: drivers/usb/storage/usb-storage: 'USB_STORAGE' exported > > twice. Previous export was in drivers/usb/storage/usb-storage.ko > > ERROR: "usb_stor_set_xfer_buf" [drivers/usb/storage/ums-usbat.ko] undefined! > > ERROR: "usb_stor_access_xfer_buf" [drivers/usb/storage/ums-usbat.ko] > > undefined! > > ERROR: "usb_stor_post_reset" [drivers/usb/storage/ums-usbat.ko] undefined! > > ERROR: "usb_stor_disconnect" [drivers/usb/storage/ums-usbat.ko] undefined! > > > > ERROR: "usb_stor_adjust_quirks" [drivers/usb/storage/uas.ko] undefined! > > ERROR: "usb_stor_sense_invalidCDB" [drivers/usb/storage/uas.ko] undefined! > > WARNING: "USB_STORAGE" [drivers/usb/storage/usb-storage] is a static > > EXPORT_SYMBOL_GPL > > make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1 > > make: *** [Makefile:1303: modules] Error 2 > > > Hmm, I do not see those error. > I was able to build the kernel successfully. > (I asked the 0-day bot to test whole of my patch set > in case I am missing something.) > > > Could you share the steps to reproduce the errors and your .config file? >From the root of git tree - at exactly 5.4-rc1 ~/cifs-2.6$ make nsdeps CALLscripts/checksyscalls.sh CALLscripts/atomic/check-atomics.sh DESCEND objtool CHK include/generated/compile.h CHK kernel/kheaders_data.tar.xz Building modules, stage 2. MODPOST 5340 modules Building modules, stage 2. MODPOST 5340 modules ./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name make: *** [Makefile:1710: nsdeps] Error 2 I get the same error doing "rm fs/cifs/*.o" and repeating the "make nsdeps" command I will send you the .config
Re: [PATCH] fs: cifs: mute -Wunused-const-variable message
merged into cifs-2.6.git for-next On Tue, Oct 1, 2019 at 2:34 AM Austin Kim wrote: > > After 'Initial git repository build' commit, > 'mapping_table_ERRHRD' variable has not been used. > > So 'mapping_table_ERRHRD' const variable could be removed > to mute below warning message: > >fs/cifs/netmisc.c:120:40: warning: unused variable 'mapping_table_ERRHRD' > [-Wunused-const-variable] >static const struct smb_to_posix_error mapping_table_ERRHRD[] = { >^ > Signed-off-by: Austin Kim > --- > fs/cifs/netmisc.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c > index 49c17ee1..9b41436 100644 > --- a/fs/cifs/netmisc.c > +++ b/fs/cifs/netmisc.c > @@ -117,10 +117,6 @@ static const struct smb_to_posix_error > mapping_table_ERRSRV[] = { > {0, 0} > }; > > -static const struct smb_to_posix_error mapping_table_ERRHRD[] = { > - {0, 0} > -}; > - > /* > * Convert a string containing text IPv4 or IPv6 address to binary form. > * > -- > 2.6.2 > -- Thanks, Steve
Re: nsdeps not working on modules in 5.4-rc1
On Thu, Oct 3, 2019 at 10:24 AM Masahiro Yamada wrote: > > Hi Steve, > > On Fri, Oct 4, 2019 at 12:15 AM Steve French wrote: > > > > On Thu, Oct 3, 2019 at 5:43 AM Matthias Maennich > > wrote: > > > > > > Hi Steve! > > > > > > On Wed, Oct 02, 2019 at 06:54:26PM -0500, Steve French wrote: > > > >And running the build differently, from the root of the git tree > > > >(5.4-rc1) rather than using the Ubuntu 5.4-rc1 headers also fails > > > > > > > >e.g. "make M=fs/cifs modules nsdeps" > > > > > > > >... > > > > LD [M] fs/cifs/cifs.o > > > > Building modules, stage 2. > > > > MODPOST 1 modules > > > >WARNING: module cifs uses symbol sigprocmask from namespace > > > >_fs/cifs/cache.o), but does not import it. > > > >... > > > >WARNING: module cifs uses symbol posix_test_lock from namespace > > > >cifs/cache.o), but does not import it. > > > > CC [M] fs/cifs/cifs.mod.o > > > > LD [M] fs/cifs/cifs.ko > > > > Building modules, stage 2. > > > > MODPOST 1 modules > > > >./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name > > > >make: *** [Makefile:1710: nsdeps] Error 2 > > > > > > Thanks for reporting this. It appears to me you hit a bug that was > > > recently discovered: when building with `make M=some/subdirectory`, > > > modpost is misbehaving. Can you try whether this patch series solves > > > your problems: > > > https://lore.kernel.org/lkml/20191003075826.7478-1-yamada.masah...@socionext.com/ > > > In particular patch 2/6 out of the series. > > > > > > Cheers, > > > Matthias > > > > > > Applying just patch 2 and doing "make" from the root of the git tree > > (5.4-rc1), at the tail end of the build I got > > > > ... > > Kernel: arch/x86/boot/bzImage is ready (#87) > > Building modules, stage 2. > > MODPOST 5340 modules > > free(): invalid pointer > > Aborted (core dumped) > > > Right. > > Since 2/6 depends on 1/6, > applying only the second one does not work. Applying both 1 and 2 I get the following error doing make (although it makes it a long way into the build) WARNING: drivers/usb/storage/usb-storage: 'USB_STORAGE' exported twice. Previous export was in drivers/usb/storage/usb-storage.ko ERROR: "usb_stor_set_xfer_buf" [drivers/usb/storage/ums-usbat.ko] undefined! ERROR: "usb_stor_access_xfer_buf" [drivers/usb/storage/ums-usbat.ko] undefined! ERROR: "usb_stor_post_reset" [drivers/usb/storage/ums-usbat.ko] undefined! ERROR: "usb_stor_disconnect" [drivers/usb/storage/ums-usbat.ko] undefined! ERROR: "usb_stor_adjust_quirks" [drivers/usb/storage/uas.ko] undefined! ERROR: "usb_stor_sense_invalidCDB" [drivers/usb/storage/uas.ko] undefined! WARNING: "USB_STORAGE" [drivers/usb/storage/usb-storage] is a static EXPORT_SYMBOL_GPL make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1 make: *** [Makefile:1303: modules] Error 2 Running "make M=fs/cifs nsdeps" I still get the error Building modules, stage 2. MODPOST 1 modules ./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name make: *** [Makefile:1710: nsdeps] Error 2 -- Thanks, Steve
Re: nsdeps not working on modules in 5.4-rc1
On Thu, Oct 3, 2019 at 5:43 AM Matthias Maennich wrote: > > Hi Steve! > > On Wed, Oct 02, 2019 at 06:54:26PM -0500, Steve French wrote: > >And running the build differently, from the root of the git tree > >(5.4-rc1) rather than using the Ubuntu 5.4-rc1 headers also fails > > > >e.g. "make M=fs/cifs modules nsdeps" > > > >... > > LD [M] fs/cifs/cifs.o > > Building modules, stage 2. > > MODPOST 1 modules > >WARNING: module cifs uses symbol sigprocmask from namespace > >_fs/cifs/cache.o), but does not import it. > >... > >WARNING: module cifs uses symbol posix_test_lock from namespace > >cifs/cache.o), but does not import it. > > CC [M] fs/cifs/cifs.mod.o > > LD [M] fs/cifs/cifs.ko > > Building modules, stage 2. > > MODPOST 1 modules > >./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name > >make: *** [Makefile:1710: nsdeps] Error 2 > > Thanks for reporting this. It appears to me you hit a bug that was > recently discovered: when building with `make M=some/subdirectory`, > modpost is misbehaving. Can you try whether this patch series solves > your problems: > https://lore.kernel.org/lkml/20191003075826.7478-1-yamada.masah...@socionext.com/ > In particular patch 2/6 out of the series. > > Cheers, > Matthias Applying just patch 2 and doing "make" from the root of the git tree (5.4-rc1), at the tail end of the build I got ... Kernel: arch/x86/boot/bzImage is ready (#87) Building modules, stage 2. MODPOST 5340 modules free(): invalid pointer Aborted (core dumped) make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 134 make: *** [Makefile:1303: modules] Error 2 With patch 2 and doing make M=fs/cifs nsdeps from the root of the git tree I get $ make M=fs/cifs nsdeps Building modules, stage 2. MODPOST 1 modules Building modules, stage 2. MODPOST 1 modules ./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name make: *** [Makefile:1710: nsdeps] Error 2 -- Thanks, Steve
Re: nsdeps not working on modules in 5.4-rc1
And running the build differently, from the root of the git tree (5.4-rc1) rather than using the Ubuntu 5.4-rc1 headers also fails e.g. "make M=fs/cifs modules nsdeps" ... LD [M] fs/cifs/cifs.o Building modules, stage 2. MODPOST 1 modules WARNING: module cifs uses symbol sigprocmask from namespace _fs/cifs/cache.o), but does not import it. ... WARNING: module cifs uses symbol posix_test_lock from namespace cifs/cache.o), but does not import it. CC [M] fs/cifs/cifs.mod.o LD [M] fs/cifs/cifs.ko Building modules, stage 2. MODPOST 1 modules ./scripts/nsdeps: 34: local: ./fs/cifs/cifsfs.c: bad variable name make: *** [Makefile:1710: nsdeps] Error 2 On Wed, Oct 2, 2019 at 6:45 PM Steve French wrote: > > Following the instructions in Documentation/namespaces to autogenerate > the namespace changes to avoid the multiple build warnings in 5.4-rc1 > for my module ... I am not able to get nsdeps to work. For example > in my module directory (fs/cifs) trying to build with nsdeps: > > make -C /usr/src/linux-headers-`uname -r` M=`pwd` modules nsdeps > > gets the error "cat: ./modules.order: No such file or directory" > > This is on Ubuntu 18, running current 5.4-rc1 kernel. It looks like > it is looking for modules.order in the wrong directory (it is present > in fs/cifs - but it looks like it is looking for it in /usr/src where > of course it won't be found) > > I am trying to remove the hundreds of new warnings introduced by > namespaces in 5.4-rc1 when building my module e.g. > > WARNING: module cifs uses symbol __fscache_acquire_cookie from > namespace .o: $(deps_/home/sfrench/cifs-2.6/fs/cifs/cache.o), but does > not import it. > -- > Thanks, > > Steve -- Thanks, Steve
nsdeps not working on modules in 5.4-rc1
Following the instructions in Documentation/namespaces to autogenerate the namespace changes to avoid the multiple build warnings in 5.4-rc1 for my module ... I am not able to get nsdeps to work. For example in my module directory (fs/cifs) trying to build with nsdeps: make -C /usr/src/linux-headers-`uname -r` M=`pwd` modules nsdeps gets the error "cat: ./modules.order: No such file or directory" This is on Ubuntu 18, running current 5.4-rc1 kernel. It looks like it is looking for modules.order in the wrong directory (it is present in fs/cifs - but it looks like it is looking for it in /usr/src where of course it won't be found) I am trying to remove the hundreds of new warnings introduced by namespaces in 5.4-rc1 when building my module e.g. WARNING: module cifs uses symbol __fscache_acquire_cookie from namespace .o: $(deps_/home/sfrench/cifs-2.6/fs/cifs/cache.o), but does not import it. -- Thanks, Steve