From b61f7dcf4eb2653e870c9079b02d11a0834cfe39 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 27 Apr 2018 20:46:22 +0100 Subject: [PATCH 01/14] afs: Fix directory page locking The afs directory loading code (primarily afs_read_dir()) locks all the pages that hold a directory's content blob to defend against getdents/getdents races and getdents/lookup races where the competitors issue conflicting reads on the same data. As the reads will complete consecutively, they may retrieve different versions of the data and one may overwrite the data that the other is busy parsing. Fix this by not locking the pages at all, but rather by turning the validation lock into an rwsem and getting an exclusive lock on it whilst reading the data or validating the attributes and a shared lock whilst parsing the data. Sharing the attribute validation lock should be fine as the data fetch will retrieve the attributes also. The individual page locks aren't needed at all as the only place they're being used is to serialise data loading. Without this patch, the: if (!test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags)) { ... } part of afs_read_dir() may be skipped, leaving the pages unlocked when we hit the success: clause - in which case we try to unlock the not-locked pages, leading to the following oops: page:ffffe38b405b4300 count:3 mapcount:0 mapping:ffff98156c83a978 index:0x0 flags: 0xfffe000001004(referenced|private) raw: 000fffe000001004 ffff98156c83a978 0000000000000000 00000003ffffffff raw: dead000000000100 dead000000000200 0000000000000001 ffff98156b27c000 page dumped because: VM_BUG_ON_PAGE(!PageLocked(page)) page->mem_cgroup:ffff98156b27c000 ------------[ cut here ]------------ kernel BUG at mm/filemap.c:1205! ... RIP: 0010:unlock_page+0x43/0x50 ... Call Trace: afs_dir_iterate+0x789/0x8f0 [kafs] ? _cond_resched+0x15/0x30 ? kmem_cache_alloc_trace+0x166/0x1d0 ? afs_do_lookup+0x69/0x490 [kafs] ? afs_do_lookup+0x101/0x490 [kafs] ? key_default_cmp+0x20/0x20 ? request_key+0x3c/0x80 ? afs_lookup+0xf1/0x340 [kafs] ? __lookup_slow+0x97/0x150 ? lookup_slow+0x35/0x50 ? walk_component+0x1bf/0x490 ? path_lookupat.isra.52+0x75/0x200 ? filename_lookup.part.66+0xa0/0x170 ? afs_end_vnode_operation+0x41/0x60 [kafs] ? __check_object_size+0x9c/0x171 ? strncpy_from_user+0x4a/0x170 ? vfs_statx+0x73/0xe0 ? __do_sys_newlstat+0x39/0x70 ? __x64_sys_getdents+0xc9/0x140 ? __x64_sys_getdents+0x140/0x140 ? do_syscall_64+0x5b/0x160 ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: f3ddee8dc4e2 ("afs: Fix directory handling") Reported-by: Marc Dionne Signed-off-by: David Howells --- fs/afs/dir.c | 38 ++++++++++++++++++-------------------- fs/afs/inode.c | 6 +++--- fs/afs/internal.h | 2 +- fs/afs/super.c | 2 +- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/fs/afs/dir.c b/fs/afs/dir.c index 5889f70d4d27..2853acd64482 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -180,6 +180,7 @@ static int afs_dir_open(struct inode *inode, struct file *file) * get reclaimed during the iteration. */ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key) + __acquires(&dvnode->validate_lock) { struct afs_read *req; loff_t i_size; @@ -261,18 +262,21 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key) /* If we're going to reload, we need to lock all the pages to prevent * races. */ + ret = -ERESTARTSYS; + if (down_read_killable(&dvnode->validate_lock) < 0) + goto error; + + if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags)) + goto success; + + up_read(&dvnode->validate_lock); + if (down_write_killable(&dvnode->validate_lock) < 0) + goto error; + if (!test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags)) { - ret = -ERESTARTSYS; - for (i = 0; i < req->nr_pages; i++) - if (lock_page_killable(req->pages[i]) < 0) - goto error_unlock; - - if (test_bit(AFS_VNODE_DIR_VALID, &dvnode->flags)) - goto success; - ret = afs_fetch_data(dvnode, key, req); if (ret < 0) - goto error_unlock_all; + goto error_unlock; task_io_account_read(PAGE_SIZE * req->nr_pages); @@ -284,33 +288,26 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key) for (i = 0; i < req->nr_pages; i++) if (!afs_dir_check_page(dvnode, req->pages[i], req->actual_len)) - goto error_unlock_all; + goto error_unlock; // TODO: Trim excess pages set_bit(AFS_VNODE_DIR_VALID, &dvnode->flags); } + downgrade_write(&dvnode->validate_lock); success: - i = req->nr_pages; - while (i > 0) - unlock_page(req->pages[--i]); return req; -error_unlock_all: - i = req->nr_pages; error_unlock: - while (i > 0) - unlock_page(req->pages[--i]); + up_write(&dvnode->validate_lock); error: afs_put_read(req); _leave(" = %d", ret); return ERR_PTR(ret); content_has_grown: - i = req->nr_pages; - while (i > 0) - unlock_page(req->pages[--i]); + up_write(&dvnode->validate_lock); afs_put_read(req); goto retry; } @@ -473,6 +470,7 @@ static int afs_dir_iterate(struct inode *dir, struct dir_context *ctx, } out: + up_read(&dvnode->validate_lock); afs_put_read(req); _leave(" = %d", ret); return ret; diff --git a/fs/afs/inode.c b/fs/afs/inode.c index 06194cfe9724..e855c6e5cf28 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -415,7 +415,7 @@ int afs_validate(struct afs_vnode *vnode, struct key *key) if (valid) goto valid; - mutex_lock(&vnode->validate_lock); + down_write(&vnode->validate_lock); /* if the promise has expired, we need to check the server again to get * a new promise - note that if the (parent) directory's metadata was @@ -444,13 +444,13 @@ int afs_validate(struct afs_vnode *vnode, struct key *key) * different */ if (test_and_clear_bit(AFS_VNODE_ZAP_DATA, &vnode->flags)) afs_zap_data(vnode); - mutex_unlock(&vnode->validate_lock); + up_write(&vnode->validate_lock); valid: _leave(" = 0"); return 0; error_unlock: - mutex_unlock(&vnode->validate_lock); + up_write(&vnode->validate_lock); _leave(" = %d", ret); return ret; } diff --git a/fs/afs/internal.h b/fs/afs/internal.h index f8086ec95e24..468be1e0dffb 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -494,7 +494,7 @@ struct afs_vnode { #endif struct afs_permits __rcu *permit_cache; /* cache of permits so far obtained */ struct mutex io_lock; /* Lock for serialising I/O on this mutex */ - struct mutex validate_lock; /* lock for validating this vnode */ + struct rw_semaphore validate_lock; /* lock for validating this vnode */ spinlock_t wb_lock; /* lock for wb_keys */ spinlock_t lock; /* waitqueue/flags lock */ unsigned long flags; diff --git a/fs/afs/super.c b/fs/afs/super.c index 65081ec3c36e..b02838cd9525 100644 --- a/fs/afs/super.c +++ b/fs/afs/super.c @@ -590,7 +590,7 @@ static void afs_i_init_once(void *_vnode) memset(vnode, 0, sizeof(*vnode)); inode_init_once(&vnode->vfs_inode); mutex_init(&vnode->io_lock); - mutex_init(&vnode->validate_lock); + init_rwsem(&vnode->validate_lock); spin_lock_init(&vnode->wb_lock); spin_lock_init(&vnode->lock); INIT_LIST_HEAD(&vnode->wb_keys); From 01fd79e6de74a447c5657913a335d9ce6508cdb1 Mon Sep 17 00:00:00 2001 From: David Howells Date: Wed, 9 May 2018 22:03:18 +0100 Subject: [PATCH 02/14] afs: Fix address list parsing The parsing of port specifiers in the address list obtained from the DNS resolution upcall doesn't work as in4_pton() and in6_pton() will fail on encountering an unexpected delimiter (in this case, the '+' marking the port number). However, in*_pton() can't be given multiple specifiers. Fix this by finding the delimiter in advance and not relying on in*_pton() to find the end of the address for us. Fixes: 8b2a464ced77 ("afs: Add an address list concept") Signed-off-by: David Howells --- fs/afs/addr_list.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/fs/afs/addr_list.c b/fs/afs/addr_list.c index 3bedfed608a2..7587fb665ff1 100644 --- a/fs/afs/addr_list.c +++ b/fs/afs/addr_list.c @@ -121,7 +121,7 @@ struct afs_addr_list *afs_parse_text_addrs(const char *text, size_t len, p = text; do { struct sockaddr_rxrpc *srx = &alist->addrs[alist->nr_addrs]; - char tdelim = delim; + const char *q, *stop; if (*p == delim) { p++; @@ -130,28 +130,33 @@ struct afs_addr_list *afs_parse_text_addrs(const char *text, size_t len, if (*p == '[') { p++; - tdelim = ']'; + q = memchr(p, ']', end - p); + } else { + for (q = p; q < end; q++) + if (*q == '+' || *q == delim) + break; } - if (in4_pton(p, end - p, + if (in4_pton(p, q - p, (u8 *)&srx->transport.sin6.sin6_addr.s6_addr32[3], - tdelim, &p)) { + -1, &stop)) { srx->transport.sin6.sin6_addr.s6_addr32[0] = 0; srx->transport.sin6.sin6_addr.s6_addr32[1] = 0; srx->transport.sin6.sin6_addr.s6_addr32[2] = htonl(0xffff); - } else if (in6_pton(p, end - p, + } else if (in6_pton(p, q - p, srx->transport.sin6.sin6_addr.s6_addr, - tdelim, &p)) { + -1, &stop)) { /* Nothing to do */ } else { goto bad_address; } - if (tdelim == ']') { - if (p == end || *p != ']') - goto bad_address; + if (stop != q) + goto bad_address; + + p = q; + if (q < end && *q == ']') p++; - } if (p < end) { if (*p == '+') { From f2686b09269ec1a6f23028b5675d87c3b4579a4c Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 10 May 2018 14:12:50 +0100 Subject: [PATCH 03/14] afs: Fix giving up callbacks on server destruction When a server record is destroyed, we want to send a message to the server telling it that we're giving up all the callbacks it has promised us. Apply two fixes to this: (1) Only send the FS.GiveUpAllCallBacks message if we actually got a callback from that server. We assume this to be the case if we performed at least one successful FS operation on that server. (2) Send it to the address last used for that server rather than always picking the first address in the list (which might be unreachable). Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation") Signed-off-by: David Howells --- fs/afs/internal.h | 1 + fs/afs/rxrpc.c | 6 +++++- fs/afs/server.c | 8 +++++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 468be1e0dffb..8de04b29bec1 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -396,6 +396,7 @@ struct afs_server { #define AFS_SERVER_FL_PROBED 5 /* The fileserver has been probed */ #define AFS_SERVER_FL_PROBING 6 /* Fileserver is being probed */ #define AFS_SERVER_FL_NO_IBULK 7 /* Fileserver doesn't support FS.InlineBulkStatus */ +#define AFS_SERVER_FL_MAY_HAVE_CB 8 /* May have callbacks on this fileserver */ atomic_t usage; u32 addr_version; /* Address list version */ diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c index 5c6263972ec9..1f6235a6e9ae 100644 --- a/fs/afs/rxrpc.c +++ b/fs/afs/rxrpc.c @@ -482,8 +482,12 @@ static void afs_deliver_to_call(struct afs_call *call) state = READ_ONCE(call->state); switch (ret) { case 0: - if (state == AFS_CALL_CL_PROC_REPLY) + if (state == AFS_CALL_CL_PROC_REPLY) { + if (call->cbi) + set_bit(AFS_SERVER_FL_MAY_HAVE_CB, + &call->cbi->server->flags); goto call_complete; + } ASSERTCMP(state, >, AFS_CALL_CL_PROC_REPLY); goto done; case -EINPROGRESS: diff --git a/fs/afs/server.c b/fs/afs/server.c index 629c74986cff..2c5cff60e34d 100644 --- a/fs/afs/server.c +++ b/fs/afs/server.c @@ -395,14 +395,16 @@ static void afs_destroy_server(struct afs_net *net, struct afs_server *server) struct afs_addr_list *alist = rcu_access_pointer(server->addresses); struct afs_addr_cursor ac = { .alist = alist, - .addr = &alist->addrs[0], .start = alist->index, - .index = alist->index, + .index = 0, + .addr = &alist->addrs[alist->index], .error = 0, }; _enter("%p", server); - afs_fs_give_up_all_callbacks(net, server, &ac, NULL); + if (test_bit(AFS_SERVER_FL_MAY_HAVE_CB, &server->flags)) + afs_fs_give_up_all_callbacks(net, server, &ac, NULL); + call_rcu(&server->rcu, afs_server_rcu); afs_dec_servers_outstanding(net); } From d4a96bec7a7362834ef5c31d7b2cc9bf36eb0570 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 10 May 2018 08:43:04 +0100 Subject: [PATCH 04/14] afs: Fix refcounting in callback registration The refcounting on afs_cb_interest struct objects in afs_register_server_cb_interest() is wrong as it uses the server list entry's call back interest pointer without regard for the fact that it might be replaced at any time and the object thrown away. Fix this by: (1) Put a lock on the afs_server_list struct that can be used to mediate access to the callback interest pointers in the servers array. (2) Keep a ref on the callback interest that we get from the entry. (3) Dropping the old reference held by vnode->cb_interest if we replace the pointer. Fixes: c435ee34551e ("afs: Overhaul the callback handling") Signed-off-by: David Howells --- fs/afs/callback.c | 64 ++++++++++++++++++++++++++++++-------------- fs/afs/internal.h | 7 +++-- fs/afs/rotate.c | 4 +-- fs/afs/server_list.c | 7 +++-- 4 files changed, 56 insertions(+), 26 deletions(-) diff --git a/fs/afs/callback.c b/fs/afs/callback.c index abd9a84f4e88..09332945d322 100644 --- a/fs/afs/callback.c +++ b/fs/afs/callback.c @@ -23,36 +23,55 @@ /* * Set up an interest-in-callbacks record for a volume on a server and * register it with the server. - * - Called with volume->server_sem held. + * - Called with vnode->io_lock held. */ int afs_register_server_cb_interest(struct afs_vnode *vnode, - struct afs_server_entry *entry) + struct afs_server_list *slist, + unsigned int index) { - struct afs_cb_interest *cbi = entry->cb_interest, *vcbi, *new, *x; + struct afs_server_entry *entry = &slist->servers[index]; + struct afs_cb_interest *cbi, *vcbi, *new, *old; struct afs_server *server = entry->server; again: + if (vnode->cb_interest && + likely(vnode->cb_interest == entry->cb_interest)) + return 0; + + read_lock(&slist->lock); + cbi = afs_get_cb_interest(entry->cb_interest); + read_unlock(&slist->lock); + vcbi = vnode->cb_interest; if (vcbi) { - if (vcbi == cbi) - return 0; - - if (cbi && vcbi->server == cbi->server) { - write_seqlock(&vnode->cb_lock); - vnode->cb_interest = afs_get_cb_interest(cbi); - write_sequnlock(&vnode->cb_lock); + if (vcbi == cbi) { afs_put_cb_interest(afs_v2net(vnode), cbi); return 0; } + /* Use a new interest in the server list for the same server + * rather than an old one that's still attached to a vnode. + */ + if (cbi && vcbi->server == cbi->server) { + write_seqlock(&vnode->cb_lock); + old = vnode->cb_interest; + vnode->cb_interest = cbi; + write_sequnlock(&vnode->cb_lock); + afs_put_cb_interest(afs_v2net(vnode), old); + return 0; + } + + /* Re-use the one attached to the vnode. */ if (!cbi && vcbi->server == server) { - afs_get_cb_interest(vcbi); - x = cmpxchg(&entry->cb_interest, cbi, vcbi); - if (x != cbi) { - cbi = x; - afs_put_cb_interest(afs_v2net(vnode), vcbi); + write_lock(&slist->lock); + if (entry->cb_interest) { + write_unlock(&slist->lock); + afs_put_cb_interest(afs_v2net(vnode), cbi); goto again; } + + entry->cb_interest = cbi; + write_unlock(&slist->lock); return 0; } } @@ -72,13 +91,16 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode, list_add_tail(&new->cb_link, &server->cb_interests); write_unlock(&server->cb_break_lock); - x = cmpxchg(&entry->cb_interest, cbi, new); - if (x == cbi) { + write_lock(&slist->lock); + if (!entry->cb_interest) { + entry->cb_interest = afs_get_cb_interest(new); cbi = new; + new = NULL; } else { - cbi = x; - afs_put_cb_interest(afs_v2net(vnode), new); + cbi = afs_get_cb_interest(entry->cb_interest); } + write_unlock(&slist->lock); + afs_put_cb_interest(afs_v2net(vnode), new); } ASSERT(cbi); @@ -88,11 +110,13 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode, */ write_seqlock(&vnode->cb_lock); - vnode->cb_interest = afs_get_cb_interest(cbi); + old = vnode->cb_interest; + vnode->cb_interest = cbi; vnode->cb_s_break = cbi->server->cb_s_break; clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags); write_sequnlock(&vnode->cb_lock); + afs_put_cb_interest(afs_v2net(vnode), old); return 0; } diff --git a/fs/afs/internal.h b/fs/afs/internal.h index 8de04b29bec1..e75e57e13320 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -434,6 +434,7 @@ struct afs_server_list { unsigned short index; /* Server currently in use */ unsigned short vnovol_mask; /* Servers to be skipped due to VNOVOL */ unsigned int seq; /* Set to ->servers_seq when installed */ + rwlock_t lock; struct afs_server_entry servers[]; }; @@ -649,13 +650,15 @@ extern void afs_init_callback_state(struct afs_server *); extern void afs_break_callback(struct afs_vnode *); extern void afs_break_callbacks(struct afs_server *, size_t, struct afs_callback_break*); -extern int afs_register_server_cb_interest(struct afs_vnode *, struct afs_server_entry *); +extern int afs_register_server_cb_interest(struct afs_vnode *, + struct afs_server_list *, unsigned int); extern void afs_put_cb_interest(struct afs_net *, struct afs_cb_interest *); extern void afs_clear_callback_interests(struct afs_net *, struct afs_server_list *); static inline struct afs_cb_interest *afs_get_cb_interest(struct afs_cb_interest *cbi) { - refcount_inc(&cbi->usage); + if (cbi) + refcount_inc(&cbi->usage); return cbi; } diff --git a/fs/afs/rotate.c b/fs/afs/rotate.c index ac0feac9d746..4a26d51b2968 100644 --- a/fs/afs/rotate.c +++ b/fs/afs/rotate.c @@ -350,8 +350,8 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) * break request before we've finished decoding the reply and * installing the vnode. */ - fc->ac.error = afs_register_server_cb_interest( - vnode, &fc->server_list->servers[fc->index]); + fc->ac.error = afs_register_server_cb_interest(vnode, fc->server_list, + fc->index); if (fc->ac.error < 0) goto failed; diff --git a/fs/afs/server_list.c b/fs/afs/server_list.c index 0f8dc4c8f07c..8a5760aa5832 100644 --- a/fs/afs/server_list.c +++ b/fs/afs/server_list.c @@ -49,6 +49,7 @@ struct afs_server_list *afs_alloc_server_list(struct afs_cell *cell, goto error; refcount_set(&slist->usage, 1); + rwlock_init(&slist->lock); /* Make sure a records exists for each server in the list. */ for (i = 0; i < vldb->nr_servers; i++) { @@ -64,9 +65,11 @@ struct afs_server_list *afs_alloc_server_list(struct afs_cell *cell, goto error_2; } - /* Insertion-sort by server pointer */ + /* Insertion-sort by UUID */ for (j = 0; j < slist->nr_servers; j++) - if (slist->servers[j].server >= server) + if (memcmp(&slist->servers[j].server->uuid, + &server->uuid, + sizeof(server->uuid)) >= 0) break; if (j < slist->nr_servers) { if (slist->servers[j].server == server) { From ec5a3b4b507efca903d848518dcf2ebf7b04b466 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 10 May 2018 14:22:38 +0100 Subject: [PATCH 05/14] afs: Fix server rotation's handling of fileserver probe failure The server rotation algorithm just gives up if it fails to probe a fileserver. Fix this by rotating to the next fileserver instead. Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation") Signed-off-by: David Howells --- fs/afs/rotate.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fs/afs/rotate.c b/fs/afs/rotate.c index 4a26d51b2968..84584dcced72 100644 --- a/fs/afs/rotate.c +++ b/fs/afs/rotate.c @@ -369,8 +369,16 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) if (!test_bit(AFS_SERVER_FL_PROBED, &server->flags)) { fc->ac.alist = afs_get_addrlist(alist); - if (!afs_probe_fileserver(fc)) - goto failed; + if (!afs_probe_fileserver(fc)) { + switch (fc->ac.error) { + case -ENOMEM: + case -ERESTARTSYS: + case -EINTR: + goto failed; + default: + goto next_server; + } + } } if (!fc->ac.alist) From 684b0f68cf1c1cf4a40834818653491c5cad4435 Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 10 May 2018 21:51:47 +0100 Subject: [PATCH 06/14] afs: Fix AFSFetchStatus decoder to provide OpenAFS compatibility The OpenAFS server's RXAFS_InlineBulkStatus implementation has a bug whereby if an error occurs on one of the vnodes being queried, then the errorCode field is set correctly in the corresponding status, but the interfaceVersion field is left unset. Fix kAFS to deal with this by evaluating the AFSFetchStatus blob against the following cases when called from FS.InlineBulkStatus delivery: (1) If InterfaceVersion == 0 then: (a) If errorCode != 0 then it indicates the abort code for the corresponding vnode. (b) If errorCode == 0 then the status record is invalid. (2) If InterfaceVersion == 1 then: (a) If errorCode != 0 then it indicates the abort code for the corresponding vnode. (b) If errorCode == 0 then the status record is valid and can be parsed. (3) If InterfaceVersion is anything else then the status record is invalid. Fixes: dd9fbcb8e103 ("afs: Rearrange status mapping") Reported-by: Jeffrey Altman Signed-off-by: David Howells --- fs/afs/fsclient.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c index efacdb7c1dee..d16f26c6cdbe 100644 --- a/fs/afs/fsclient.c +++ b/fs/afs/fsclient.c @@ -134,6 +134,7 @@ static int xdr_decode_AFSFetchStatus(struct afs_call *call, struct afs_read *read_req) { const struct afs_xdr_AFSFetchStatus *xdr = (const void *)*_bp; + bool inline_error = (call->operation_ID == afs_FS_InlineBulkStatus); u64 data_version, size; u32 type, abort_code; u8 flags = 0; @@ -142,13 +143,32 @@ static int xdr_decode_AFSFetchStatus(struct afs_call *call, if (vnode) write_seqlock(&vnode->cb_lock); + abort_code = ntohl(xdr->abort_code); + if (xdr->if_version != htonl(AFS_FSTATUS_VERSION)) { + if (xdr->if_version == htonl(0) && + abort_code != 0 && + inline_error) { + /* The OpenAFS fileserver has a bug in FS.InlineBulkStatus + * whereby it doesn't set the interface version in the error + * case. + */ + status->abort_code = abort_code; + ret = 0; + goto out; + } + pr_warn("Unknown AFSFetchStatus version %u\n", ntohl(xdr->if_version)); goto bad; } + if (abort_code != 0 && inline_error) { + status->abort_code = abort_code; + ret = 0; + goto out; + } + type = ntohl(xdr->type); - abort_code = ntohl(xdr->abort_code); switch (type) { case AFS_FTYPE_FILE: case AFS_FTYPE_DIR: @@ -165,13 +185,6 @@ static int xdr_decode_AFSFetchStatus(struct afs_call *call, } status->type = type; break; - case AFS_FTYPE_INVALID: - if (abort_code != 0) { - status->abort_code = abort_code; - ret = 0; - goto out; - } - /* Fall through */ default: goto bad; } From 3d9fa91161387ee629e7a07c47934d119910c8ae Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 May 2018 22:55:59 +0100 Subject: [PATCH 07/14] afs: Fix VNOVOL handling in address rotation If a volume location record lists multiple file servers for a volume, then it's possible that due to a misconfiguration or a changing configuration that one of the file servers doesn't know about it yet and will abort VNOVOL. Currently, the rotation algorithm will stop with EREMOTEIO. Fix this by moving on to try the next server if VNOVOL is returned. Once all the servers have been tried and the record rechecked, the algorithm will stop with EREMOTEIO or ENOMEDIUM. Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation") Reported-by: Marc Dionne Signed-off-by: David Howells --- fs/afs/rotate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/afs/rotate.c b/fs/afs/rotate.c index 84584dcced72..e065bc0768e6 100644 --- a/fs/afs/rotate.c +++ b/fs/afs/rotate.c @@ -179,7 +179,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) */ if (fc->flags & AFS_FS_CURSOR_VNOVOL) { fc->ac.error = -EREMOTEIO; - goto failed; + goto next_server; } write_lock(&vnode->volume->servers_lock); @@ -201,7 +201,7 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc) */ if (vnode->volume->servers == fc->server_list) { fc->ac.error = -EREMOTEIO; - goto failed; + goto next_server; } /* Try again */ From 001ab5a67ee5d191c64aebf4b4ef8c7a0dcfd2bc Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 May 2018 23:21:35 +0100 Subject: [PATCH 08/14] afs: Fix the handling of CB.InitCallBackState3 to find the server by UUID Fix the handling of the CB.InitCallBackState3 service call to find the record of a server that we're using by looking it up by the UUID passed as the parameter rather than by its address (of which it might have many, and which may change). Fixes: c35eccb1f614 ("[AFS]: Implement the CB.InitCallBackState3 operation.") Signed-off-by: David Howells --- fs/afs/cmservice.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c index 357de908df3a..bcd13397bd59 100644 --- a/fs/afs/cmservice.c +++ b/fs/afs/cmservice.c @@ -341,7 +341,6 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call) */ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call) { - struct sockaddr_rxrpc srx; struct afs_server *server; struct afs_uuid *r; unsigned loop; @@ -398,8 +397,9 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call) /* we'll need the file server record as that tells us which set of * vnodes to operate upon */ - rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx); - server = afs_find_server(call->net, &srx); + rcu_read_lock(); + server = afs_find_server_by_uuid(call->net, call->request); + rcu_read_unlock(); if (!server) return -ENOTCONN; call->cm_server = server; From 3709a399c15e4273d9a94b123374f12e5664318c Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 May 2018 22:59:42 +0100 Subject: [PATCH 09/14] afs: Add a tracepoint to record callbacks from unlisted servers Add a tracepoint to record callbacks from servers for which we don't have a record. Signed-off-by: David Howells --- fs/afs/cmservice.c | 12 ++++++++--- include/trace/events/afs.h | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c index bcd13397bd59..9f13375f49b8 100644 --- a/fs/afs/cmservice.c +++ b/fs/afs/cmservice.c @@ -287,8 +287,10 @@ static int afs_deliver_cb_callback(struct afs_call *call) * vnodes to operate upon */ rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx); server = afs_find_server(call->net, &srx); - if (!server) + if (!server) { + trace_afs_cm_no_server(call, &srx); return -ENOTCONN; + } call->cm_server = server; return afs_queue_call_work(call); @@ -329,8 +331,10 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call) /* we'll need the file server record as that tells us which set of * vnodes to operate upon */ server = afs_find_server(call->net, &srx); - if (!server) + if (!server) { + trace_afs_cm_no_server(call, &srx); return -ENOTCONN; + } call->cm_server = server; return afs_queue_call_work(call); @@ -400,8 +404,10 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call) rcu_read_lock(); server = afs_find_server_by_uuid(call->net, call->request); rcu_read_unlock(); - if (!server) + if (!server) { + trace_afs_cm_no_server_u(call, call->request); return -ENOTCONN; + } call->cm_server = server; return afs_queue_call_work(call); diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h index f0820554caa9..d0a341bc4540 100644 --- a/include/trace/events/afs.h +++ b/include/trace/events/afs.h @@ -575,6 +575,48 @@ TRACE_EVENT(afs_protocol_error, __entry->call, __entry->error, __entry->where) ); +TRACE_EVENT(afs_cm_no_server, + TP_PROTO(struct afs_call *call, struct sockaddr_rxrpc *srx), + + TP_ARGS(call, srx), + + TP_STRUCT__entry( + __field(unsigned int, call ) + __field(unsigned int, op_id ) + __field_struct(struct sockaddr_rxrpc, srx ) + ), + + TP_fast_assign( + __entry->call = call->debug_id; + __entry->op_id = call->operation_ID; + memcpy(&__entry->srx, srx, sizeof(__entry->srx)); + ), + + TP_printk("c=%08x op=%u %pISpc", + __entry->call, __entry->op_id, &__entry->srx.transport) + ); + +TRACE_EVENT(afs_cm_no_server_u, + TP_PROTO(struct afs_call *call, const uuid_t *uuid), + + TP_ARGS(call, uuid), + + TP_STRUCT__entry( + __field(unsigned int, call ) + __field(unsigned int, op_id ) + __field_struct(uuid_t, uuid ) + ), + + TP_fast_assign( + __entry->call = call->debug_id; + __entry->op_id = call->operation_ID; + memcpy(&__entry->uuid, uuid, sizeof(__entry->uuid)); + ), + + TP_printk("c=%08x op=%u %pU", + __entry->call, __entry->op_id, &__entry->uuid) + ); + #endif /* _TRACE_AFS_H */ /* This part must be outside protection */ From a86b06d1ccd218a6a50d6a3a88fbd2abcd0eaa94 Mon Sep 17 00:00:00 2001 From: David Howells Date: Fri, 11 May 2018 23:45:40 +0100 Subject: [PATCH 10/14] afs: Fix the handling of an unfound server in CM operations If the client cache manager operations that need the server record (CB.Callback, CB.InitCallBackState, and CB.InitCallBackState3) can't find the server record, they abort the call from the file server with RX_CALL_DEAD when they should return okay. Fixes: c35eccb1f614 ("[AFS]: Implement the CB.InitCallBackState3 operation.") Signed-off-by: David Howells --- fs/afs/cmservice.c | 34 ++++++++++++---------------------- fs/afs/rxrpc.c | 5 ----- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c index 9f13375f49b8..b44491410af3 100644 --- a/fs/afs/cmservice.c +++ b/fs/afs/cmservice.c @@ -143,8 +143,8 @@ static void afs_cm_destructor(struct afs_call *call) * received. The step number here must match the final number in * afs_deliver_cb_callback(). */ - if (call->unmarshall == 5) { - ASSERT(call->cm_server && call->count && call->request); + if (call->cm_server && call->unmarshall == 5) { + ASSERT(call->count && call->request); afs_break_callbacks(call->cm_server, call->count, call->request); } @@ -168,7 +168,8 @@ static void SRXAFSCB_CallBack(struct work_struct *work) * yet */ afs_send_empty_reply(call); - afs_break_callbacks(call->cm_server, call->count, call->request); + if (call->cm_server) + afs_break_callbacks(call->cm_server, call->count, call->request); afs_put_call(call); _leave(""); } @@ -180,7 +181,6 @@ static int afs_deliver_cb_callback(struct afs_call *call) { struct afs_callback_break *cb; struct sockaddr_rxrpc srx; - struct afs_server *server; __be32 *bp; int ret, loop; @@ -286,12 +286,9 @@ static int afs_deliver_cb_callback(struct afs_call *call) /* we'll need the file server record as that tells us which set of * vnodes to operate upon */ rxrpc_kernel_get_peer(call->net->socket, call->rxcall, &srx); - server = afs_find_server(call->net, &srx); - if (!server) { + call->cm_server = afs_find_server(call->net, &srx); + if (!call->cm_server) trace_afs_cm_no_server(call, &srx); - return -ENOTCONN; - } - call->cm_server = server; return afs_queue_call_work(call); } @@ -305,7 +302,8 @@ static void SRXAFSCB_InitCallBackState(struct work_struct *work) _enter("{%p}", call->cm_server); - afs_init_callback_state(call->cm_server); + if (call->cm_server) + afs_init_callback_state(call->cm_server); afs_send_empty_reply(call); afs_put_call(call); _leave(""); @@ -317,7 +315,6 @@ static void SRXAFSCB_InitCallBackState(struct work_struct *work) static int afs_deliver_cb_init_call_back_state(struct afs_call *call) { struct sockaddr_rxrpc srx; - struct afs_server *server; int ret; _enter(""); @@ -330,12 +327,9 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call) /* we'll need the file server record as that tells us which set of * vnodes to operate upon */ - server = afs_find_server(call->net, &srx); - if (!server) { + call->cm_server = afs_find_server(call->net, &srx); + if (!call->cm_server) trace_afs_cm_no_server(call, &srx); - return -ENOTCONN; - } - call->cm_server = server; return afs_queue_call_work(call); } @@ -345,7 +339,6 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call) */ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call) { - struct afs_server *server; struct afs_uuid *r; unsigned loop; __be32 *b; @@ -402,13 +395,10 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call) /* we'll need the file server record as that tells us which set of * vnodes to operate upon */ rcu_read_lock(); - server = afs_find_server_by_uuid(call->net, call->request); + call->cm_server = afs_find_server_by_uuid(call->net, call->request); rcu_read_unlock(); - if (!server) { + if (!call->cm_server) trace_afs_cm_no_server_u(call, call->request); - return -ENOTCONN; - } - call->cm_server = server; return afs_queue_call_work(call); } diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c index 1f6235a6e9ae..d0eee5d32c94 100644 --- a/fs/afs/rxrpc.c +++ b/fs/afs/rxrpc.c @@ -497,11 +497,6 @@ static void afs_deliver_to_call(struct afs_call *call) case -ECONNABORTED: ASSERTCMP(state, ==, AFS_CALL_COMPLETE); goto done; - case -ENOTCONN: - abort_code = RX_CALL_DEAD; - rxrpc_kernel_abort_call(call->net->socket, call->rxcall, - abort_code, ret, "KNC"); - goto local_abort; case -ENOTSUPP: abort_code = RXGEN_OPCODE; rxrpc_kernel_abort_call(call->net->socket, call->rxcall, From f9c1bba3d392843f046d2ee27b4dfcec989d8a4b Mon Sep 17 00:00:00 2001 From: Marc Dionne Date: Fri, 11 May 2018 21:35:06 -0300 Subject: [PATCH 11/14] afs: Fix afs_find_server search loop The code that looks up servers by addresses makes the assumption that the list of addresses for a server is sorted. It exits the loop if it finds that the target address is larger than the current candidate. As the list is not currently sorted, this can lead to a failure to find a matching server, which can cause callbacks from that server to be ignored. Remove the early exit case so that the complete list is searched. Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation") Signed-off-by: Marc Dionne Signed-off-by: David Howells --- fs/afs/server.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/fs/afs/server.c b/fs/afs/server.c index 2c5cff60e34d..3af4625e2f8c 100644 --- a/fs/afs/server.c +++ b/fs/afs/server.c @@ -67,12 +67,6 @@ struct afs_server *afs_find_server(struct afs_net *net, sizeof(struct in6_addr)); if (diff == 0) goto found; - if (diff < 0) { - // TODO: Sort the list - //if (i == alist->nr_ipv4) - // goto not_found; - break; - } } } } else { @@ -87,17 +81,10 @@ struct afs_server *afs_find_server(struct afs_net *net, (u32 __force)b->sin6_addr.s6_addr32[3]); if (diff == 0) goto found; - if (diff < 0) { - // TODO: Sort the list - //if (i == 0) - // goto not_found; - break; - } } } } - //not_found: server = NULL; found: if (server && !atomic_inc_not_zero(&server->usage)) From 68251f0a6818f3be19b1471f36c956ca97c1427d Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 12 May 2018 22:31:33 +0100 Subject: [PATCH 12/14] afs: Fix whole-volume callback handling It's possible for an AFS file server to issue a whole-volume notification that callbacks on all the vnodes in the file have been broken. This is done for R/O and backup volumes (which don't have per-file callbacks) and for things like a volume being taken offline. Fix callback handling to detect whole-volume notifications, to track it across operations and to check it during inode validation. Fixes: c435ee34551e ("afs: Overhaul the callback handling") Signed-off-by: David Howells --- fs/afs/callback.c | 28 +++++++++++++++++++++------- fs/afs/dir.c | 18 +++++++++--------- fs/afs/file.c | 2 +- fs/afs/flock.c | 6 +++--- fs/afs/fsclient.c | 2 +- fs/afs/inode.c | 13 ++++++++----- fs/afs/internal.h | 15 +++++++++++++++ fs/afs/security.c | 7 +++---- fs/afs/super.c | 2 +- fs/afs/write.c | 2 +- 10 files changed, 63 insertions(+), 32 deletions(-) diff --git a/fs/afs/callback.c b/fs/afs/callback.c index 09332945d322..571437dcb252 100644 --- a/fs/afs/callback.c +++ b/fs/afs/callback.c @@ -113,6 +113,7 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode, old = vnode->cb_interest; vnode->cb_interest = cbi; vnode->cb_s_break = cbi->server->cb_s_break; + vnode->cb_v_break = vnode->volume->cb_v_break; clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags); write_sequnlock(&vnode->cb_lock); @@ -195,13 +196,24 @@ static void afs_break_one_callback(struct afs_server *server, if (cbi->vid != fid->vid) continue; - data.volume = NULL; - data.fid = *fid; - inode = ilookup5_nowait(cbi->sb, fid->vnode, afs_iget5_test, &data); - if (inode) { - vnode = AFS_FS_I(inode); - afs_break_callback(vnode); - iput(inode); + if (fid->vnode == 0 && fid->unique == 0) { + /* The callback break applies to an entire volume. */ + struct afs_super_info *as = AFS_FS_S(cbi->sb); + struct afs_volume *volume = as->volume; + + write_lock(&volume->cb_break_lock); + volume->cb_v_break++; + write_unlock(&volume->cb_break_lock); + } else { + data.volume = NULL; + data.fid = *fid; + inode = ilookup5_nowait(cbi->sb, fid->vnode, + afs_iget5_test, &data); + if (inode) { + vnode = AFS_FS_I(inode); + afs_break_callback(vnode); + iput(inode); + } } } @@ -219,6 +231,8 @@ void afs_break_callbacks(struct afs_server *server, size_t count, ASSERT(server != NULL); ASSERTCMP(count, <=, AFSCBMAX); + /* TODO: Sort the callback break list by volume ID */ + for (; count > 0; callbacks++, count--) { _debug("- Fid { vl=%08x n=%u u=%u } CB { v=%u x=%u t=%u }", callbacks->fid.vid, diff --git a/fs/afs/dir.c b/fs/afs/dir.c index 2853acd64482..7d623008157f 100644 --- a/fs/afs/dir.c +++ b/fs/afs/dir.c @@ -1141,7 +1141,7 @@ static int afs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, dvnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); afs_fs_create(&fc, dentry->d_name.name, mode, data_version, &newfid, &newstatus, &newcb); } @@ -1211,7 +1211,7 @@ static int afs_rmdir(struct inode *dir, struct dentry *dentry) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, dvnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); afs_fs_remove(&fc, dentry->d_name.name, true, data_version); } @@ -1314,7 +1314,7 @@ static int afs_unlink(struct inode *dir, struct dentry *dentry) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, dvnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); afs_fs_remove(&fc, dentry->d_name.name, false, data_version); } @@ -1371,7 +1371,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode, ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, dvnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); afs_fs_create(&fc, dentry->d_name.name, mode, data_version, &newfid, &newstatus, &newcb); } @@ -1441,8 +1441,8 @@ static int afs_link(struct dentry *from, struct inode *dir, } while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; - fc.cb_break_2 = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); + fc.cb_break_2 = afs_calc_vnode_cb_break(vnode); afs_fs_link(&fc, vnode, dentry->d_name.name, data_version); } @@ -1510,7 +1510,7 @@ static int afs_symlink(struct inode *dir, struct dentry *dentry, ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, dvnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = dvnode->cb_break + dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(dvnode); afs_fs_symlink(&fc, dentry->d_name.name, content, data_version, &newfid, &newstatus); @@ -1586,8 +1586,8 @@ static int afs_rename(struct inode *old_dir, struct dentry *old_dentry, } } while (afs_select_fileserver(&fc)) { - fc.cb_break = orig_dvnode->cb_break + orig_dvnode->cb_s_break; - fc.cb_break_2 = new_dvnode->cb_break + new_dvnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(orig_dvnode); + fc.cb_break_2 = afs_calc_vnode_cb_break(new_dvnode); afs_fs_rename(&fc, old_dentry->d_name.name, new_dvnode, new_dentry->d_name.name, orig_data_version, new_data_version); diff --git a/fs/afs/file.c b/fs/afs/file.c index c24c08016dd9..7d4f26198573 100644 --- a/fs/afs/file.c +++ b/fs/afs/file.c @@ -238,7 +238,7 @@ int afs_fetch_data(struct afs_vnode *vnode, struct key *key, struct afs_read *de ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_fetch_data(&fc, desc); } diff --git a/fs/afs/flock.c b/fs/afs/flock.c index 7a0e017070ec..dc62d15a964b 100644 --- a/fs/afs/flock.c +++ b/fs/afs/flock.c @@ -86,7 +86,7 @@ static int afs_set_lock(struct afs_vnode *vnode, struct key *key, ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_set_lock(&fc, type); } @@ -117,7 +117,7 @@ static int afs_extend_lock(struct afs_vnode *vnode, struct key *key) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_current_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_extend_lock(&fc); } @@ -148,7 +148,7 @@ static int afs_release_lock(struct afs_vnode *vnode, struct key *key) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_current_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_release_lock(&fc); } diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c index d16f26c6cdbe..b273e1d60478 100644 --- a/fs/afs/fsclient.c +++ b/fs/afs/fsclient.c @@ -261,7 +261,7 @@ static void xdr_decode_AFSCallBack(struct afs_call *call, write_seqlock(&vnode->cb_lock); - if (call->cb_break == (vnode->cb_break + cbi->server->cb_s_break)) { + if (call->cb_break == afs_cb_break_sum(vnode, cbi)) { vnode->cb_version = ntohl(*bp++); cb_expiry = ntohl(*bp++); vnode->cb_type = ntohl(*bp++); diff --git a/fs/afs/inode.c b/fs/afs/inode.c index e855c6e5cf28..479b7fdda124 100644 --- a/fs/afs/inode.c +++ b/fs/afs/inode.c @@ -108,7 +108,7 @@ int afs_fetch_status(struct afs_vnode *vnode, struct key *key, bool new_inode) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_fetch_file_status(&fc, NULL, new_inode); } @@ -393,15 +393,18 @@ int afs_validate(struct afs_vnode *vnode, struct key *key) read_seqlock_excl(&vnode->cb_lock); if (test_bit(AFS_VNODE_CB_PROMISED, &vnode->flags)) { - if (vnode->cb_s_break != vnode->cb_interest->server->cb_s_break) { + if (vnode->cb_s_break != vnode->cb_interest->server->cb_s_break || + vnode->cb_v_break != vnode->volume->cb_v_break) { vnode->cb_s_break = vnode->cb_interest->server->cb_s_break; + vnode->cb_v_break = vnode->volume->cb_v_break; + valid = false; } else if (vnode->status.type == AFS_FTYPE_DIR && test_bit(AFS_VNODE_DIR_VALID, &vnode->flags) && vnode->cb_expires_at - 10 > now) { - valid = true; + valid = true; } else if (!test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags) && vnode->cb_expires_at - 10 > now) { - valid = true; + valid = true; } } else if (test_bit(AFS_VNODE_DELETED, &vnode->flags)) { valid = true; @@ -574,7 +577,7 @@ int afs_setattr(struct dentry *dentry, struct iattr *attr) ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_setattr(&fc, attr); } diff --git a/fs/afs/internal.h b/fs/afs/internal.h index e75e57e13320..e3f8a46663db 100644 --- a/fs/afs/internal.h +++ b/fs/afs/internal.h @@ -461,6 +461,9 @@ struct afs_volume { rwlock_t servers_lock; /* Lock for ->servers */ unsigned int servers_seq; /* Incremented each time ->servers changes */ + unsigned cb_v_break; /* Break-everything counter. */ + rwlock_t cb_break_lock; + afs_voltype_t type; /* type of volume */ short error; char type_force; /* force volume type (suppress R/O -> R/W) */ @@ -521,6 +524,7 @@ struct afs_vnode { /* outstanding callback notification on this file */ struct afs_cb_interest *cb_interest; /* Server on which this resides */ unsigned int cb_s_break; /* Mass break counter on ->server */ + unsigned int cb_v_break; /* Mass break counter on ->volume */ unsigned int cb_break; /* Break counter on vnode */ seqlock_t cb_lock; /* Lock for ->cb_interest, ->status, ->cb_*break */ @@ -662,6 +666,17 @@ static inline struct afs_cb_interest *afs_get_cb_interest(struct afs_cb_interest return cbi; } +static inline unsigned int afs_calc_vnode_cb_break(struct afs_vnode *vnode) +{ + return vnode->cb_break + vnode->cb_s_break + vnode->cb_v_break; +} + +static inline unsigned int afs_cb_break_sum(struct afs_vnode *vnode, + struct afs_cb_interest *cbi) +{ + return vnode->cb_break + cbi->server->cb_s_break + vnode->volume->cb_v_break; +} + /* * cell.c */ diff --git a/fs/afs/security.c b/fs/afs/security.c index cea2fff313dc..1992b0ffa543 100644 --- a/fs/afs/security.c +++ b/fs/afs/security.c @@ -147,8 +147,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key, break; } - if (cb_break != (vnode->cb_break + - vnode->cb_interest->server->cb_s_break)) { + if (cb_break != afs_cb_break_sum(vnode, vnode->cb_interest)) { changed = true; break; } @@ -178,7 +177,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key, } } - if (cb_break != (vnode->cb_break + vnode->cb_interest->server->cb_s_break)) + if (cb_break != afs_cb_break_sum(vnode, vnode->cb_interest)) goto someone_else_changed_it; /* We need a ref on any permits list we want to copy as we'll have to @@ -257,7 +256,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key, spin_lock(&vnode->lock); zap = rcu_access_pointer(vnode->permit_cache); - if (cb_break == (vnode->cb_break + vnode->cb_interest->server->cb_s_break) && + if (cb_break == afs_cb_break_sum(vnode, vnode->cb_interest) && zap == permits) rcu_assign_pointer(vnode->permit_cache, replacement); else diff --git a/fs/afs/super.c b/fs/afs/super.c index b02838cd9525..9e5d7966621c 100644 --- a/fs/afs/super.c +++ b/fs/afs/super.c @@ -688,7 +688,7 @@ static int afs_statfs(struct dentry *dentry, struct kstatfs *buf) if (afs_begin_vnode_operation(&fc, vnode, key)) { fc.flags |= AFS_FS_CURSOR_NO_VSLEEP; while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_get_volume_status(&fc, &vs); } diff --git a/fs/afs/write.c b/fs/afs/write.c index c164698dc304..8b39e6ebb40b 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -351,7 +351,7 @@ static int afs_store_data(struct address_space *mapping, ret = -ERESTARTSYS; if (afs_begin_vnode_operation(&fc, vnode, wbk->key)) { while (afs_select_fileserver(&fc)) { - fc.cb_break = vnode->cb_break + vnode->cb_s_break; + fc.cb_break = afs_calc_vnode_cb_break(vnode); afs_fs_store_data(&fc, mapping, first, last, offset, to); } From 428edade4e6c70e5b51fcd4188d944fbb744d84c Mon Sep 17 00:00:00 2001 From: David Howells Date: Sat, 12 May 2018 00:28:58 +0100 Subject: [PATCH 13/14] afs: Fix CB.CallBack handling The handling of CB.CallBack messages sent by the fileserver to the client is broken in that they are currently being processed after the reply has been transmitted. This is not what the fileserver expects, however. It holds up change visibility until the reply comes so as to maintain cache coherency, and so expects the client to have to refetch the state on the affected files. Fix CB.CallBack handling to perform the callback break before sending the reply. The fileserver is free to hold up status fetches issued by other threads on the same client that occur in reponse to the callback until any pending changes have been committed. Fixes: d001648ec7cf ("rxrpc: Don't expose skbs to in-kernel users [ver #2]") Signed-off-by: David Howells --- fs/afs/cmservice.c | 35 +++++++---------------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c index b44491410af3..c332c95a6940 100644 --- a/fs/afs/cmservice.c +++ b/fs/afs/cmservice.c @@ -133,21 +133,10 @@ bool afs_cm_incoming_call(struct afs_call *call) } /* - * clean up a cache manager call + * Clean up a cache manager call. */ static void afs_cm_destructor(struct afs_call *call) { - _enter(""); - - /* Break the callbacks here so that we do it after the final ACK is - * received. The step number here must match the final number in - * afs_deliver_cb_callback(). - */ - if (call->cm_server && call->unmarshall == 5) { - ASSERT(call->count && call->request); - afs_break_callbacks(call->cm_server, call->count, call->request); - } - kfree(call->buffer); call->buffer = NULL; } @@ -161,15 +150,14 @@ static void SRXAFSCB_CallBack(struct work_struct *work) _enter(""); - /* be sure to send the reply *before* attempting to spam the AFS server - * with FSFetchStatus requests on the vnodes with broken callbacks lest - * the AFS server get into a vicious cycle of trying to break further - * callbacks because it hadn't received completion of the CBCallBack op - * yet */ - afs_send_empty_reply(call); - + /* We need to break the callbacks before sending the reply as the + * server holds up change visibility till it receives our reply so as + * to maintain cache coherency. + */ if (call->cm_server) afs_break_callbacks(call->cm_server, call->count, call->request); + + afs_send_empty_reply(call); afs_put_call(call); _leave(""); } @@ -267,15 +255,6 @@ static int afs_deliver_cb_callback(struct afs_call *call) call->offset = 0; call->unmarshall++; - - /* Record that the message was unmarshalled successfully so - * that the call destructor can know do the callback breaking - * work, even if the final ACK isn't received. - * - * If the step number changes, then afs_cm_destructor() must be - * updated also. - */ - call->unmarshall++; case 5: break; } From 4776cab43fd3111618112737a257dc3ef368eddd Mon Sep 17 00:00:00 2001 From: David Howells Date: Thu, 10 May 2018 23:10:40 +0100 Subject: [PATCH 14/14] afs: Fix the non-encryption of calls Some AFS servers refuse to accept unencrypted traffic, so can't be accessed with kAFS. Set the AF_RXRPC security level to encrypt client calls to deal with this. Note that incoming service calls are set by the remote client and so aren't affected by this. This requires an AF_RXRPC patch to pass the value set by setsockopt to calls begun by the kernel. Signed-off-by: David Howells --- fs/afs/rxrpc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c index d0eee5d32c94..08735948f15d 100644 --- a/fs/afs/rxrpc.c +++ b/fs/afs/rxrpc.c @@ -41,6 +41,7 @@ int afs_open_socket(struct afs_net *net) { struct sockaddr_rxrpc srx; struct socket *socket; + unsigned int min_level; int ret; _enter(""); @@ -60,6 +61,12 @@ int afs_open_socket(struct afs_net *net) srx.transport.sin6.sin6_family = AF_INET6; srx.transport.sin6.sin6_port = htons(AFS_CM_PORT); + min_level = RXRPC_SECURITY_ENCRYPT; + ret = kernel_setsockopt(socket, SOL_RXRPC, RXRPC_MIN_SECURITY_LEVEL, + (void *)&min_level, sizeof(min_level)); + if (ret < 0) + goto error_2; + ret = kernel_bind(socket, (struct sockaddr *) &srx, sizeof(srx)); if (ret == -EADDRINUSE) { srx.transport.sin6.sin6_port = 0;