diff options
| -rw-r--r-- | drivers/xen/xenbus/xenbus_dev_frontend.c | 4 | ||||
| -rw-r--r-- | fs/open.c | 18 | ||||
| -rw-r--r-- | fs/read_write.c | 5 | ||||
| -rw-r--r-- | include/linux/fs.h | 4 | ||||
| -rw-r--r-- | scripts/coccinelle/api/stream_open.cocci | 363 |
5 files changed, 389 insertions, 5 deletions
diff --git a/drivers/xen/xenbus/xenbus_dev_frontend.c b/drivers/xen/xenbus/xenbus_dev_frontend.c index c3e201025ef0..0782ff3c2273 100644 --- a/drivers/xen/xenbus/xenbus_dev_frontend.c +++ b/drivers/xen/xenbus/xenbus_dev_frontend.c | |||
| @@ -622,9 +622,7 @@ static int xenbus_file_open(struct inode *inode, struct file *filp) | |||
| 622 | if (xen_store_evtchn == 0) | 622 | if (xen_store_evtchn == 0) |
| 623 | return -ENOENT; | 623 | return -ENOENT; |
| 624 | 624 | ||
| 625 | nonseekable_open(inode, filp); | 625 | stream_open(inode, filp); |
| 626 | |||
| 627 | filp->f_mode &= ~FMODE_ATOMIC_POS; /* cdev-style semantics */ | ||
| 628 | 626 | ||
| 629 | u = kzalloc(sizeof(*u), GFP_KERNEL); | 627 | u = kzalloc(sizeof(*u), GFP_KERNEL); |
| 630 | if (u == NULL) | 628 | if (u == NULL) |
| @@ -1215,3 +1215,21 @@ int nonseekable_open(struct inode *inode, struct file *filp) | |||
| 1215 | } | 1215 | } |
| 1216 | 1216 | ||
| 1217 | EXPORT_SYMBOL(nonseekable_open); | 1217 | EXPORT_SYMBOL(nonseekable_open); |
| 1218 | |||
| 1219 | /* | ||
| 1220 | * stream_open is used by subsystems that want stream-like file descriptors. | ||
| 1221 | * Such file descriptors are not seekable and don't have notion of position | ||
| 1222 | * (file.f_pos is always 0). Contrary to file descriptors of other regular | ||
| 1223 | * files, .read() and .write() can run simultaneously. | ||
| 1224 | * | ||
| 1225 | * stream_open never fails and is marked to return int so that it could be | ||
| 1226 | * directly used as file_operations.open . | ||
| 1227 | */ | ||
| 1228 | int stream_open(struct inode *inode, struct file *filp) | ||
| 1229 | { | ||
| 1230 | filp->f_mode &= ~(FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE | FMODE_ATOMIC_POS); | ||
| 1231 | filp->f_mode |= FMODE_STREAM; | ||
| 1232 | return 0; | ||
| 1233 | } | ||
| 1234 | |||
| 1235 | EXPORT_SYMBOL(stream_open); | ||
diff --git a/fs/read_write.c b/fs/read_write.c index 177ccc3d405a..61b43ad7608e 100644 --- a/fs/read_write.c +++ b/fs/read_write.c | |||
| @@ -560,12 +560,13 @@ ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_ | |||
| 560 | 560 | ||
| 561 | static inline loff_t file_pos_read(struct file *file) | 561 | static inline loff_t file_pos_read(struct file *file) |
| 562 | { | 562 | { |
| 563 | return file->f_pos; | 563 | return file->f_mode & FMODE_STREAM ? 0 : file->f_pos; |
| 564 | } | 564 | } |
| 565 | 565 | ||
| 566 | static inline void file_pos_write(struct file *file, loff_t pos) | 566 | static inline void file_pos_write(struct file *file, loff_t pos) |
| 567 | { | 567 | { |
| 568 | file->f_pos = pos; | 568 | if ((file->f_mode & FMODE_STREAM) == 0) |
| 569 | file->f_pos = pos; | ||
| 569 | } | 570 | } |
| 570 | 571 | ||
| 571 | ssize_t ksys_read(unsigned int fd, char __user *buf, size_t count) | 572 | ssize_t ksys_read(unsigned int fd, char __user *buf, size_t count) |
diff --git a/include/linux/fs.h b/include/linux/fs.h index 8b42df09b04c..dd28e7679089 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h | |||
| @@ -158,6 +158,9 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset, | |||
| 158 | #define FMODE_OPENED ((__force fmode_t)0x80000) | 158 | #define FMODE_OPENED ((__force fmode_t)0x80000) |
| 159 | #define FMODE_CREATED ((__force fmode_t)0x100000) | 159 | #define FMODE_CREATED ((__force fmode_t)0x100000) |
| 160 | 160 | ||
| 161 | /* File is stream-like */ | ||
| 162 | #define FMODE_STREAM ((__force fmode_t)0x200000) | ||
| 163 | |||
| 161 | /* File was opened by fanotify and shouldn't generate fanotify events */ | 164 | /* File was opened by fanotify and shouldn't generate fanotify events */ |
| 162 | #define FMODE_NONOTIFY ((__force fmode_t)0x4000000) | 165 | #define FMODE_NONOTIFY ((__force fmode_t)0x4000000) |
| 163 | 166 | ||
| @@ -3074,6 +3077,7 @@ extern loff_t no_seek_end_llseek_size(struct file *, loff_t, int, loff_t); | |||
| 3074 | extern loff_t no_seek_end_llseek(struct file *, loff_t, int); | 3077 | extern loff_t no_seek_end_llseek(struct file *, loff_t, int); |
| 3075 | extern int generic_file_open(struct inode * inode, struct file * filp); | 3078 | extern int generic_file_open(struct inode * inode, struct file * filp); |
| 3076 | extern int nonseekable_open(struct inode * inode, struct file * filp); | 3079 | extern int nonseekable_open(struct inode * inode, struct file * filp); |
| 3080 | extern int stream_open(struct inode * inode, struct file * filp); | ||
| 3077 | 3081 | ||
| 3078 | #ifdef CONFIG_BLOCK | 3082 | #ifdef CONFIG_BLOCK |
| 3079 | typedef void (dio_submit_t)(struct bio *bio, struct inode *inode, | 3083 | typedef void (dio_submit_t)(struct bio *bio, struct inode *inode, |
diff --git a/scripts/coccinelle/api/stream_open.cocci b/scripts/coccinelle/api/stream_open.cocci new file mode 100644 index 000000000000..350145da7669 --- /dev/null +++ b/scripts/coccinelle/api/stream_open.cocci | |||
| @@ -0,0 +1,363 @@ | |||
| 1 | // SPDX-License-Identifier: GPL-2.0 | ||
| 2 | // Author: Kirill Smelkov (kirr@nexedi.com) | ||
| 3 | // | ||
| 4 | // Search for stream-like files that are using nonseekable_open and convert | ||
| 5 | // them to stream_open. A stream-like file is a file that does not use ppos in | ||
| 6 | // its read and write. Rationale for the conversion is to avoid deadlock in | ||
| 7 | // between read and write. | ||
| 8 | |||
| 9 | virtual report | ||
| 10 | virtual patch | ||
| 11 | virtual explain // explain decisions in the patch (SPFLAGS="-D explain") | ||
| 12 | |||
| 13 | // stream-like reader & writer - ones that do not depend on f_pos. | ||
| 14 | @ stream_reader @ | ||
| 15 | identifier readstream, ppos; | ||
| 16 | identifier f, buf, len; | ||
| 17 | type loff_t; | ||
| 18 | @@ | ||
| 19 | ssize_t readstream(struct file *f, char *buf, size_t len, loff_t *ppos) | ||
| 20 | { | ||
| 21 | ... when != ppos | ||
| 22 | } | ||
| 23 | |||
| 24 | @ stream_writer @ | ||
| 25 | identifier writestream, ppos; | ||
| 26 | identifier f, buf, len; | ||
| 27 | type loff_t; | ||
| 28 | @@ | ||
| 29 | ssize_t writestream(struct file *f, const char *buf, size_t len, loff_t *ppos) | ||
| 30 | { | ||
| 31 | ... when != ppos | ||
| 32 | } | ||
| 33 | |||
| 34 | |||
| 35 | // a function that blocks | ||
| 36 | @ blocks @ | ||
| 37 | identifier block_f; | ||
| 38 | identifier wait_event =~ "^wait_event_.*"; | ||
| 39 | @@ | ||
| 40 | block_f(...) { | ||
| 41 | ... when exists | ||
| 42 | wait_event(...) | ||
| 43 | ... when exists | ||
| 44 | } | ||
| 45 | |||
| 46 | // stream_reader that can block inside. | ||
| 47 | // | ||
| 48 | // XXX wait_* can be called not directly from current function (e.g. func -> f -> g -> wait()) | ||
| 49 | // XXX currently reader_blocks supports only direct and 1-level indirect cases. | ||
| 50 | @ reader_blocks_direct @ | ||
| 51 | identifier stream_reader.readstream; | ||
| 52 | identifier wait_event =~ "^wait_event_.*"; | ||
| 53 | @@ | ||
| 54 | readstream(...) | ||
| 55 | { | ||
| 56 | ... when exists | ||
| 57 | wait_event(...) | ||
| 58 | ... when exists | ||
| 59 | } | ||
| 60 | |||
| 61 | @ reader_blocks_1 @ | ||
| 62 | identifier stream_reader.readstream; | ||
| 63 | identifier blocks.block_f; | ||
| 64 | @@ | ||
| 65 | readstream(...) | ||
| 66 | { | ||
| 67 | ... when exists | ||
| 68 | block_f(...) | ||
| 69 | ... when exists | ||
| 70 | } | ||
| 71 | |||
| 72 | @ reader_blocks depends on reader_blocks_direct || reader_blocks_1 @ | ||
| 73 | identifier stream_reader.readstream; | ||
| 74 | @@ | ||
| 75 | readstream(...) { | ||
| 76 | ... | ||
| 77 | } | ||
| 78 | |||
| 79 | |||
| 80 | // file_operations + whether they have _any_ .read, .write, .llseek ... at all. | ||
| 81 | // | ||
| 82 | // XXX add support for file_operations xxx[N] = ... (sound/core/pcm_native.c) | ||
| 83 | @ fops0 @ | ||
| 84 | identifier fops; | ||
| 85 | @@ | ||
| 86 | struct file_operations fops = { | ||
| 87 | ... | ||
| 88 | }; | ||
| 89 | |||
| 90 | @ has_read @ | ||
| 91 | identifier fops0.fops; | ||
| 92 | identifier read_f; | ||
| 93 | @@ | ||
| 94 | struct file_operations fops = { | ||
| 95 | .read = read_f, | ||
| 96 | }; | ||
| 97 | |||
| 98 | @ has_read_iter @ | ||
| 99 | identifier fops0.fops; | ||
| 100 | identifier read_iter_f; | ||
| 101 | @@ | ||
| 102 | struct file_operations fops = { | ||
| 103 | .read_iter = read_iter_f, | ||
| 104 | }; | ||
| 105 | |||
| 106 | @ has_write @ | ||
| 107 | identifier fops0.fops; | ||
| 108 | identifier write_f; | ||
| 109 | @@ | ||
| 110 | struct file_operations fops = { | ||
| 111 | .write = write_f, | ||
| 112 | }; | ||
| 113 | |||
| 114 | @ has_write_iter @ | ||
| 115 | identifier fops0.fops; | ||
| 116 | identifier write_iter_f; | ||
| 117 | @@ | ||
| 118 | struct file_operations fops = { | ||
| 119 | .write_iter = write_iter_f, | ||
| 120 | }; | ||
| 121 | |||
| 122 | @ has_llseek @ | ||
| 123 | identifier fops0.fops; | ||
| 124 | identifier llseek_f; | ||
| 125 | @@ | ||
| 126 | struct file_operations fops = { | ||
| 127 | .llseek = llseek_f, | ||
| 128 | }; | ||
| 129 | |||
| 130 | @ has_no_llseek @ | ||
| 131 | identifier fops0.fops; | ||
| 132 | @@ | ||
| 133 | struct file_operations fops = { | ||
| 134 | .llseek = no_llseek, | ||
| 135 | }; | ||
| 136 | |||
| 137 | @ has_mmap @ | ||
| 138 | identifier fops0.fops; | ||
| 139 | identifier mmap_f; | ||
| 140 | @@ | ||
| 141 | struct file_operations fops = { | ||
| 142 | .mmap = mmap_f, | ||
| 143 | }; | ||
| 144 | |||
| 145 | @ has_copy_file_range @ | ||
| 146 | identifier fops0.fops; | ||
| 147 | identifier copy_file_range_f; | ||
| 148 | @@ | ||
| 149 | struct file_operations fops = { | ||
| 150 | .copy_file_range = copy_file_range_f, | ||
| 151 | }; | ||
| 152 | |||
| 153 | @ has_remap_file_range @ | ||
| 154 | identifier fops0.fops; | ||
| 155 | identifier remap_file_range_f; | ||
| 156 | @@ | ||
| 157 | struct file_operations fops = { | ||
| 158 | .remap_file_range = remap_file_range_f, | ||
| 159 | }; | ||
| 160 | |||
| 161 | @ has_splice_read @ | ||
| 162 | identifier fops0.fops; | ||
| 163 | identifier splice_read_f; | ||
| 164 | @@ | ||
| 165 | struct file_operations fops = { | ||
| 166 | .splice_read = splice_read_f, | ||
| 167 | }; | ||
| 168 | |||
| 169 | @ has_splice_write @ | ||
| 170 | identifier fops0.fops; | ||
| 171 | identifier splice_write_f; | ||
| 172 | @@ | ||
| 173 | struct file_operations fops = { | ||
| 174 | .splice_write = splice_write_f, | ||
| 175 | }; | ||
| 176 | |||
| 177 | |||
| 178 | // file_operations that is candidate for stream_open conversion - it does not | ||
| 179 | // use mmap and other methods that assume @offset access to file. | ||
| 180 | // | ||
| 181 | // XXX for simplicity require no .{read/write}_iter and no .splice_{read/write} for now. | ||
| 182 | // XXX maybe_steam.fops cannot be used in other rules - it gives "bad rule maybe_stream or bad variable fops". | ||
| 183 | @ maybe_stream depends on (!has_llseek || has_no_llseek) && !has_mmap && !has_copy_file_range && !has_remap_file_range && !has_read_iter && !has_write_iter && !has_splice_read && !has_splice_write @ | ||
| 184 | identifier fops0.fops; | ||
| 185 | @@ | ||
| 186 | struct file_operations fops = { | ||
| 187 | }; | ||
| 188 | |||
| 189 | |||
| 190 | // ---- conversions ---- | ||
| 191 | |||
| 192 | // XXX .open = nonseekable_open -> .open = stream_open | ||
| 193 | // XXX .open = func -> openfunc -> nonseekable_open | ||
| 194 | |||
| 195 | // read & write | ||
| 196 | // | ||
| 197 | // if both are used in the same file_operations together with an opener - | ||
| 198 | // under that conditions we can use stream_open instead of nonseekable_open. | ||
| 199 | @ fops_rw depends on maybe_stream @ | ||
| 200 | identifier fops0.fops, openfunc; | ||
| 201 | identifier stream_reader.readstream; | ||
| 202 | identifier stream_writer.writestream; | ||
| 203 | @@ | ||
| 204 | struct file_operations fops = { | ||
| 205 | .open = openfunc, | ||
| 206 | .read = readstream, | ||
| 207 | .write = writestream, | ||
| 208 | }; | ||
| 209 | |||
| 210 | @ report_rw depends on report @ | ||
| 211 | identifier fops_rw.openfunc; | ||
| 212 | position p1; | ||
| 213 | @@ | ||
| 214 | openfunc(...) { | ||
| 215 | <... | ||
| 216 | nonseekable_open@p1 | ||
| 217 | ...> | ||
| 218 | } | ||
| 219 | |||
| 220 | @ script:python depends on report && reader_blocks @ | ||
| 221 | fops << fops0.fops; | ||
| 222 | p << report_rw.p1; | ||
| 223 | @@ | ||
| 224 | coccilib.report.print_report(p[0], | ||
| 225 | "ERROR: %s: .read() can deadlock .write(); change nonseekable_open -> stream_open to fix." % (fops,)) | ||
| 226 | |||
| 227 | @ script:python depends on report && !reader_blocks @ | ||
| 228 | fops << fops0.fops; | ||
| 229 | p << report_rw.p1; | ||
| 230 | @@ | ||
| 231 | coccilib.report.print_report(p[0], | ||
| 232 | "WARNING: %s: .read() and .write() have stream semantic; safe to change nonseekable_open -> stream_open." % (fops,)) | ||
| 233 | |||
| 234 | |||
| 235 | @ explain_rw_deadlocked depends on explain && reader_blocks @ | ||
| 236 | identifier fops_rw.openfunc; | ||
| 237 | @@ | ||
| 238 | openfunc(...) { | ||
| 239 | <... | ||
| 240 | - nonseekable_open | ||
| 241 | + nonseekable_open /* read & write (was deadlock) */ | ||
| 242 | ...> | ||
| 243 | } | ||
| 244 | |||
| 245 | |||
| 246 | @ explain_rw_nodeadlock depends on explain && !reader_blocks @ | ||
| 247 | identifier fops_rw.openfunc; | ||
| 248 | @@ | ||
| 249 | openfunc(...) { | ||
| 250 | <... | ||
| 251 | - nonseekable_open | ||
| 252 | + nonseekable_open /* read & write (no direct deadlock) */ | ||
| 253 | ...> | ||
| 254 | } | ||
| 255 | |||
| 256 | @ patch_rw depends on patch @ | ||
| 257 | identifier fops_rw.openfunc; | ||
| 258 | @@ | ||
| 259 | openfunc(...) { | ||
| 260 | <... | ||
| 261 | - nonseekable_open | ||
| 262 | + stream_open | ||
| 263 | ...> | ||
| 264 | } | ||
| 265 | |||
| 266 | |||
| 267 | // read, but not write | ||
| 268 | @ fops_r depends on maybe_stream && !has_write @ | ||
| 269 | identifier fops0.fops, openfunc; | ||
| 270 | identifier stream_reader.readstream; | ||
| 271 | @@ | ||
| 272 | struct file_operations fops = { | ||
| 273 | .open = openfunc, | ||
| 274 | .read = readstream, | ||
| 275 | }; | ||
| 276 | |||
| 277 | @ report_r depends on report @ | ||
| 278 | identifier fops_r.openfunc; | ||
| 279 | position p1; | ||
| 280 | @@ | ||
| 281 | openfunc(...) { | ||
| 282 | <... | ||
| 283 | nonseekable_open@p1 | ||
| 284 | ...> | ||
| 285 | } | ||
| 286 | |||
| 287 | @ script:python depends on report @ | ||
| 288 | fops << fops0.fops; | ||
| 289 | p << report_r.p1; | ||
| 290 | @@ | ||
| 291 | coccilib.report.print_report(p[0], | ||
| 292 | "WARNING: %s: .read() has stream semantic; safe to change nonseekable_open -> stream_open." % (fops,)) | ||
| 293 | |||
| 294 | @ explain_r depends on explain @ | ||
| 295 | identifier fops_r.openfunc; | ||
| 296 | @@ | ||
| 297 | openfunc(...) { | ||
| 298 | <... | ||
| 299 | - nonseekable_open | ||
| 300 | + nonseekable_open /* read only */ | ||
| 301 | ...> | ||
| 302 | } | ||
| 303 | |||
| 304 | @ patch_r depends on patch @ | ||
| 305 | identifier fops_r.openfunc; | ||
| 306 | @@ | ||
| 307 | openfunc(...) { | ||
| 308 | <... | ||
| 309 | - nonseekable_open | ||
| 310 | + stream_open | ||
| 311 | ...> | ||
| 312 | } | ||
| 313 | |||
| 314 | |||
| 315 | // write, but not read | ||
| 316 | @ fops_w depends on maybe_stream && !has_read @ | ||
| 317 | identifier fops0.fops, openfunc; | ||
| 318 | identifier stream_writer.writestream; | ||
| 319 | @@ | ||
| 320 | struct file_operations fops = { | ||
| 321 | .open = openfunc, | ||
| 322 | .write = writestream, | ||
| 323 | }; | ||
| 324 | |||
| 325 | @ report_w depends on report @ | ||
| 326 | identifier fops_w.openfunc; | ||
| 327 | position p1; | ||
| 328 | @@ | ||
| 329 | openfunc(...) { | ||
| 330 | <... | ||
| 331 | nonseekable_open@p1 | ||
| 332 | ...> | ||
| 333 | } | ||
| 334 | |||
| 335 | @ script:python depends on report @ | ||
| 336 | fops << fops0.fops; | ||
| 337 | p << report_w.p1; | ||
| 338 | @@ | ||
| 339 | coccilib.report.print_report(p[0], | ||
| 340 | "WARNING: %s: .write() has stream semantic; safe to change nonseekable_open -> stream_open." % (fops,)) | ||
| 341 | |||
| 342 | @ explain_w depends on explain @ | ||
| 343 | identifier fops_w.openfunc; | ||
| 344 | @@ | ||
| 345 | openfunc(...) { | ||
| 346 | <... | ||
| 347 | - nonseekable_open | ||
| 348 | + nonseekable_open /* write only */ | ||
| 349 | ...> | ||
| 350 | } | ||
| 351 | |||
| 352 | @ patch_w depends on patch @ | ||
| 353 | identifier fops_w.openfunc; | ||
| 354 | @@ | ||
| 355 | openfunc(...) { | ||
| 356 | <... | ||
| 357 | - nonseekable_open | ||
| 358 | + stream_open | ||
| 359 | ...> | ||
| 360 | } | ||
| 361 | |||
| 362 | |||
| 363 | // no read, no write - don't change anything | ||
