[PATCH] fuse: fix bitfield race

Fix race in setting bitfields of fuse_conn. Spotted by Andrew Morton.

The two fields ->connected and ->mounted were always changed with the
fuse_lock held. But other bitfields in the same structure were changed
without the lock. In theory this could lead to losing the assignment of
even the ones under lock. The chosen solution is to change these two
fields to be a full unsigned type. The other bitfields aren't "important"
enough to warrant the extra complexity of full locking or changing them to
bitops.

For all bitfields document why they are safe wrt. concurrent
assignments.

Also make the initialization of the 'num_waiting' atomic counter explicit.

Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

authored by Miklos Szeredi and committed by Linus Torvalds 095da6cb bacac382

+23 -3
+21 -3
fs/fuse/fuse_i.h
··· 94 /** Header returned from userspace */ 95 struct fuse_out_header h; 96 97 /** Last argument is variable length (can be shorter than 98 arg->size) */ 99 unsigned argvar:1; ··· 140 141 /** refcount */ 142 atomic_t count; 143 144 /** True if the request has reply */ 145 unsigned isreply:1; ··· 261 u64 reqctr; 262 263 /** Mount is active */ 264 - unsigned mounted : 1; 265 266 /** Connection established, cleared on umount, connection 267 abort and device release */ 268 - unsigned connected : 1; 269 270 - /** Connection failed (version mismatch) */ 271 unsigned conn_error : 1; 272 273 /** Is fsync not implemented by fs? */ 274 unsigned no_fsync : 1;
··· 94 /** Header returned from userspace */ 95 struct fuse_out_header h; 96 97 + /* 98 + * The following bitfields are not changed during the request 99 + * processing 100 + */ 101 + 102 /** Last argument is variable length (can be shorter than 103 arg->size) */ 104 unsigned argvar:1; ··· 135 136 /** refcount */ 137 atomic_t count; 138 + 139 + /* 140 + * The following bitfields are either set once before the 141 + * request is queued or setting/clearing them is protected by 142 + * fuse_lock 143 + */ 144 145 /** True if the request has reply */ 146 unsigned isreply:1; ··· 250 u64 reqctr; 251 252 /** Mount is active */ 253 + unsigned mounted; 254 255 /** Connection established, cleared on umount, connection 256 abort and device release */ 257 + unsigned connected; 258 259 + /** Connection failed (version mismatch). Cannot race with 260 + setting other bitfields since it is only set once in INIT 261 + reply, before any other request, and never cleared */ 262 unsigned conn_error : 1; 263 + 264 + /* 265 + * The following bitfields are only for optimization purposes 266 + * and hence races in setting them will not cause malfunction 267 + */ 268 269 /** Is fsync not implemented by fs? */ 270 unsigned no_fsync : 1;
+2
fs/fuse/inode.c
··· 397 init_rwsem(&fc->sbput_sem); 398 kobj_set_kset_s(fc, connections_subsys); 399 kobject_init(&fc->kobj); 400 for (i = 0; i < FUSE_MAX_OUTSTANDING; i++) { 401 struct fuse_req *req = fuse_request_alloc(); 402 if (!req) { ··· 493 to be exactly one request available */ 494 struct fuse_req *req = fuse_get_request(fc); 495 struct fuse_init_in *arg = &req->misc.init_in; 496 arg->major = FUSE_KERNEL_VERSION; 497 arg->minor = FUSE_KERNEL_MINOR_VERSION; 498 req->in.h.opcode = FUSE_INIT;
··· 397 init_rwsem(&fc->sbput_sem); 398 kobj_set_kset_s(fc, connections_subsys); 399 kobject_init(&fc->kobj); 400 + atomic_set(&fc->num_waiting, 0); 401 for (i = 0; i < FUSE_MAX_OUTSTANDING; i++) { 402 struct fuse_req *req = fuse_request_alloc(); 403 if (!req) { ··· 492 to be exactly one request available */ 493 struct fuse_req *req = fuse_get_request(fc); 494 struct fuse_init_in *arg = &req->misc.init_in; 495 + 496 arg->major = FUSE_KERNEL_VERSION; 497 arg->minor = FUSE_KERNEL_MINOR_VERSION; 498 req->in.h.opcode = FUSE_INIT;