Pull 9p updates from Dominique Martinet: - improve p9_check_errors to check buffer size instead of msize when possible (e.g. not zero-copy) - some more syzbot and KCSAN fixes - minor headers include cleanup * tag '9p-for-6.2-rc1' of https://github.com/martinetd/linux: 9p/client: fix data race on req->status net/9p: fix response size check in p9_check_errors() net/9p: distinguish zero-copy requests 9p/xen: do not memcpy header into req->rc 9p: set req refcount to zero to avoid uninitialized usage 9p/net: Remove unneeded idr.h #include 9p/fs: Remove unneeded idr.h #include
Unified · Split
| @@ -11,7 +11,6 @@ · 10 unmodified lines | ||
| 11 | 11 | #include <linux/fs.h> |
| 12 | 12 | #include <linux/slab.h> |
| 13 | 13 | #include <linux/sched.h> |
| 14 | −#include <linux/idr.h> | |
| 15 | 14 | #include <net/9p/9p.h> |
| 16 | 15 | #include <net/9p/client.h> |
| 17 | 16 | |
| @@ -14,7 +14,6 @@ · 13 unmodified lines | ||
| 14 | 14 | #include <linux/sched.h> |
| 15 | 15 | #include <linux/cred.h> |
| 16 | 16 | #include <linux/parser.h> |
| 17 | −#include <linux/idr.h> | |
| 18 | 17 | #include <linux/slab.h> |
| 19 | 18 | #include <linux/seq_file.h> |
| 20 | 19 | #include <net/9p/9p.h> |
| @@ -14,7 +14,6 @@ · 13 unmodified lines | ||
| 14 | 14 | #include <linux/string.h> |
| 15 | 15 | #include <linux/inet.h> |
| 16 | 16 | #include <linux/pagemap.h> |
| 17 | −#include <linux/idr.h> | |
| 18 | 17 | #include <linux/sched.h> |
| 19 | 18 | #include <linux/swap.h> |
| 20 | 19 | #include <linux/uio.h> |
| @@ -15,7 +15,6 @@ · 14 unmodified lines | ||
| 15 | 15 | #include <linux/string.h> |
| 16 | 16 | #include <linux/inet.h> |
| 17 | 17 | #include <linux/namei.h> |
| 18 | −#include <linux/idr.h> | |
| 19 | 18 | #include <linux/sched.h> |
| 20 | 19 | #include <linux/slab.h> |
| 21 | 20 | #include <net/9p/9p.h> |
| @@ -14,7 +14,6 @@ · 13 unmodified lines | ||
| 14 | 14 | #include <linux/string.h> |
| 15 | 15 | #include <linux/sched.h> |
| 16 | 16 | #include <linux/inet.h> |
| 17 | −#include <linux/idr.h> | |
| 18 | 17 | #include <linux/slab.h> |
| 19 | 18 | #include <linux/uio.h> |
| 20 | 19 | #include <linux/fscache.h> |
| @@ -18,7 +18,6 @@ · 17 unmodified lines | ||
| 18 | 18 | #include <linux/pagemap.h> |
| 19 | 19 | #include <linux/utsname.h> |
| 20 | 20 | #include <linux/uaccess.h> |
| 21 | −#include <linux/idr.h> | |
| 22 | 21 | #include <linux/uio.h> |
| 23 | 22 | #include <linux/slab.h> |
| 24 | 23 | #include <net/9p/9p.h> |
| @@ -17,7 +17,6 @@ · 16 unmodified lines | ||
| 17 | 17 | #include <linux/string.h> |
| 18 | 18 | #include <linux/inet.h> |
| 19 | 19 | #include <linux/namei.h> |
| 20 | −#include <linux/idr.h> | |
| 21 | 20 | #include <linux/sched.h> |
| 22 | 21 | #include <linux/slab.h> |
| 23 | 22 | #include <linux/xattr.h> |
| @@ -15,7 +15,6 @@ · 14 unmodified lines | ||
| 15 | 15 | #include <linux/string.h> |
| 16 | 16 | #include <linux/inet.h> |
| 17 | 17 | #include <linux/namei.h> |
| 18 | −#include <linux/idr.h> | |
| 19 | 18 | #include <linux/sched.h> |
| 20 | 19 | #include <linux/slab.h> |
| 21 | 20 | #include <linux/xattr.h> |
| @@ -15,7 +15,6 @@ · 14 unmodified lines | ||
| 15 | 15 | #include <linux/inet.h> |
| 16 | 16 | #include <linux/pagemap.h> |
| 17 | 17 | #include <linux/mount.h> |
| 18 | −#include <linux/idr.h> | |
| 19 | 18 | #include <linux/sched.h> |
| 20 | 19 | #include <linux/slab.h> |
| 21 | 20 | #include <linux/statfs.h> |
| @@ -531,6 +531,7 @@ struct p9_rstatfs { · 530 unmodified lines | ||
| 531 | 531 | * @offset: used by marshalling routines to track current position in buffer |
| 532 | 532 | * @capacity: used by marshalling routines to track total malloc'd capacity |
| 533 | 533 | * @sdata: payload |
| 534 | + * @zc: whether zero-copy is used | |
| 534 | 535 | * |
| 535 | 536 | * &p9_fcall represents the structure for all 9P RPC |
| 536 | 537 | * transactions. Requests are packaged into fcalls, and reponses |
| @@ -549,6 +550,7 @@ struct p9_fcall { · 12 unmodified lines | ||
| 549 | 550 | |
| 550 | 551 | struct kmem_cache *cache; |
| 551 | 552 | u8 *sdata; |
| 553 | + bool zc; | |
| 552 | 554 | }; |
| 553 | 555 | |
| 554 | 556 | int p9_errstr2errno(char *errstr, int len); |
| @@ -297,6 +297,11 @@ p9_tag_alloc(struct p9_client *c, int8_t type, uint t_size, uint r_size, · 296 unmodified lines | ||
| 297 | 297 | p9pdu_reset(&req->rc); |
| 298 | 298 | req->t_err = 0; |
| 299 | 299 | req->status = REQ_STATUS_ALLOC; |
| 300 | + /* refcount needs to be set to 0 before inserting into the idr | |
| 301 | + * so p9_tag_lookup does not accept a request that is not fully | |
| 302 | + * initialized. refcount_set to 2 below will mark request ready. | |
| 303 | + */ | |
| 304 | + refcount_set(&req->refcount, 0); | |
| 300 | 305 | init_waitqueue_head(&req->wq); |
| 301 | 306 | INIT_LIST_HEAD(&req->req_list); |
| 302 | 307 | |
| @@ -438,7 +443,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status) · 135 unmodified lines | ||
| 438 | 443 | * the status change is visible to another thread |
| 439 | 444 | */ |
| 440 | 445 | smp_wmb(); |
| 441 | − req->status = status; | |
| 446 | + WRITE_ONCE(req->status, status); | |
| 442 | 447 | |
| 443 | 448 | wake_up(&req->wq); |
| 444 | 449 | p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag); |
| @@ -514,10 +519,9 @@ static int p9_check_errors(struct p9_client *c, struct p9_req_t *req) · 69 unmodified lines | ||
| 514 | 519 | int ecode; |
| 515 | 520 | |
| 516 | 521 | err = p9_parse_header(&req->rc, NULL, &type, NULL, 0); |
| 517 | − if (req->rc.size >= c->msize) { | |
| 518 | − p9_debug(P9_DEBUG_ERROR, | |
| 519 | − "requested packet size too big: %d\n", | |
| 520 | − req->rc.size); | |
| 522 | + if (req->rc.size > req->rc.capacity && !req->rc.zc) { | |
| 523 | + pr_err("requested packet size too big: %d does not fit %zu (type=%d)\n", | |
| 524 | + req->rc.size, req->rc.capacity, req->rc.id); | |
| 521 | 525 | return -EIO; |
| 522 | 526 | } |
| 523 | 527 | /* dump the response from server |
| @@ -600,7 +604,7 @@ static int p9_client_flush(struct p9_client *c, struct p9_req_t *oldreq) · 76 unmodified lines | ||
| 600 | 604 | /* if we haven't received a response for oldreq, |
| 601 | 605 | * remove it from the list |
| 602 | 606 | */ |
| 603 | − if (oldreq->status == REQ_STATUS_SENT) { | |
| 607 | + if (READ_ONCE(oldreq->status) == REQ_STATUS_SENT) { | |
| 604 | 608 | if (c->trans_mod->cancelled) |
| 605 | 609 | c->trans_mod->cancelled(c, oldreq); |
| 606 | 610 | } |
| @@ -680,6 +684,9 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...) · 73 unmodified lines | ||
| 680 | 684 | if (IS_ERR(req)) |
| 681 | 685 | return req; |
| 682 | 686 | |
| 687 | + req->tc.zc = false; | |
| 688 | + req->rc.zc = false; | |
| 689 | + | |
| 683 | 690 | if (signal_pending(current)) { |
| 684 | 691 | sigpending = 1; |
| 685 | 692 | clear_thread_flag(TIF_SIGPENDING); |
| @@ -697,7 +704,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...) · 11 unmodified lines | ||
| 697 | 704 | } |
| 698 | 705 | again: |
| 699 | 706 | /* Wait for the response */ |
| 700 | − err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD); | |
| 707 | + err = wait_event_killable(req->wq, | |
| 708 | + READ_ONCE(req->status) >= REQ_STATUS_RCVD); | |
| 701 | 709 | |
| 702 | 710 | /* Make sure our req is coherent with regard to updates in other |
| 703 | 711 | * threads - echoes to wmb() in the callback |
| @@ -711,7 +719,7 @@ again: · 7 unmodified lines | ||
| 711 | 719 | goto again; |
| 712 | 720 | } |
| 713 | 721 | |
| 714 | − if (req->status == REQ_STATUS_ERROR) { | |
| 722 | + if (READ_ONCE(req->status) == REQ_STATUS_ERROR) { | |
| 715 | 723 | p9_debug(P9_DEBUG_ERROR, "req_status error %d\n", req->t_err); |
| 716 | 724 | err = req->t_err; |
| 717 | 725 | } |
| @@ -724,7 +732,7 @@ again: · 6 unmodified lines | ||
| 724 | 732 | p9_client_flush(c, req); |
| 725 | 733 | |
| 726 | 734 | /* if we received the response anyway, don't signal error */ |
| 727 | − if (req->status == REQ_STATUS_RCVD) | |
| 735 | + if (READ_ONCE(req->status) == REQ_STATUS_RCVD) | |
| 728 | 736 | err = 0; |
| 729 | 737 | } |
| 730 | 738 | recalc_sigpending: |
| @@ -778,6 +786,9 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, · 47 unmodified lines | ||
| 778 | 786 | if (IS_ERR(req)) |
| 779 | 787 | return req; |
| 780 | 788 | |
| 789 | + req->tc.zc = true; | |
| 790 | + req->rc.zc = true; | |
| 791 | + | |
| 781 | 792 | if (signal_pending(current)) { |
| 782 | 793 | sigpending = 1; |
| 783 | 794 | clear_thread_flag(TIF_SIGPENDING); |
| @@ -793,7 +804,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, · 9 unmodified lines | ||
| 793 | 804 | if (err != -ERESTARTSYS) |
| 794 | 805 | goto recalc_sigpending; |
| 795 | 806 | } |
| 796 | − if (req->status == REQ_STATUS_ERROR) { | |
| 807 | + if (READ_ONCE(req->status) == REQ_STATUS_ERROR) { | |
| 797 | 808 | p9_debug(P9_DEBUG_ERROR, "req_status error %d\n", req->t_err); |
| 798 | 809 | err = req->t_err; |
| 799 | 810 | } |
| @@ -806,7 +817,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, · 6 unmodified lines | ||
| 806 | 817 | p9_client_flush(c, req); |
| 807 | 818 | |
| 808 | 819 | /* if we received the response anyway, don't signal error */ |
| 809 | − if (req->status == REQ_STATUS_RCVD) | |
| 820 | + if (READ_ONCE(req->status) == REQ_STATUS_RCVD) | |
| 810 | 821 | err = 0; |
| 811 | 822 | } |
| 812 | 823 | recalc_sigpending: |
| @@ -20,7 +20,6 @@ · 19 unmodified lines | ||
| 20 | 20 | #include <linux/un.h> |
| 21 | 21 | #include <linux/uaccess.h> |
| 22 | 22 | #include <linux/inet.h> |
| 23 | −#include <linux/idr.h> | |
| 24 | 23 | #include <linux/file.h> |
| 25 | 24 | #include <linux/parser.h> |
| 26 | 25 | #include <linux/slab.h> |
| @@ -202,11 +201,11 @@ static void p9_conn_cancel(struct p9_conn *m, int err) · 175 unmodified lines | ||
| 202 | 201 | |
| 203 | 202 | list_for_each_entry_safe(req, rtmp, &m->req_list, req_list) { |
| 204 | 203 | list_move(&req->req_list, &cancel_list); |
| 205 | − req->status = REQ_STATUS_ERROR; | |
| 204 | + WRITE_ONCE(req->status, REQ_STATUS_ERROR); | |
| 206 | 205 | } |
| 207 | 206 | list_for_each_entry_safe(req, rtmp, &m->unsent_req_list, req_list) { |
| 208 | 207 | list_move(&req->req_list, &cancel_list); |
| 209 | − req->status = REQ_STATUS_ERROR; | |
| 208 | + WRITE_ONCE(req->status, REQ_STATUS_ERROR); | |
| 210 | 209 | } |
| 211 | 210 | |
| 212 | 211 | spin_unlock(&m->req_lock); |
| @@ -467,7 +466,7 @@ static void p9_write_work(struct work_struct *work) · 254 unmodified lines | ||
| 467 | 466 | |
| 468 | 467 | req = list_entry(m->unsent_req_list.next, struct p9_req_t, |
| 469 | 468 | req_list); |
| 470 | − req->status = REQ_STATUS_SENT; | |
| 469 | + WRITE_ONCE(req->status, REQ_STATUS_SENT); | |
| 471 | 470 | p9_debug(P9_DEBUG_TRANS, "move req %p\n", req); |
| 472 | 471 | list_move_tail(&req->req_list, &m->req_list); |
| 473 | 472 | |
| @@ -676,7 +675,7 @@ static int p9_fd_request(struct p9_client *client, struct p9_req_t *req) · 202 unmodified lines | ||
| 676 | 675 | return m->err; |
| 677 | 676 | |
| 678 | 677 | spin_lock(&m->req_lock); |
| 679 | − req->status = REQ_STATUS_UNSENT; | |
| 678 | + WRITE_ONCE(req->status, REQ_STATUS_UNSENT); | |
| 680 | 679 | list_add_tail(&req->req_list, &m->unsent_req_list); |
| 681 | 680 | spin_unlock(&m->req_lock); |
| 682 | 681 | |
| @@ -703,7 +702,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req) · 20 unmodified lines | ||
| 703 | 702 | |
| 704 | 703 | if (req->status == REQ_STATUS_UNSENT) { |
| 705 | 704 | list_del(&req->req_list); |
| 706 | − req->status = REQ_STATUS_FLSHD; | |
| 705 | + WRITE_ONCE(req->status, REQ_STATUS_FLSHD); | |
| 707 | 706 | p9_req_put(client, req); |
| 708 | 707 | ret = 0; |
| 709 | 708 | } |
| @@ -732,7 +731,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req) · 22 unmodified lines | ||
| 732 | 731 | * remove it from the list. |
| 733 | 732 | */ |
| 734 | 733 | list_del(&req->req_list); |
| 735 | − req->status = REQ_STATUS_FLSHD; | |
| 734 | + WRITE_ONCE(req->status, REQ_STATUS_FLSHD); | |
| 736 | 735 | spin_unlock(&m->req_lock); |
| 737 | 736 | |
| 738 | 737 | p9_req_put(client, req); |
| @@ -21,7 +21,6 @@ · 20 unmodified lines | ||
| 21 | 21 | #include <linux/un.h> |
| 22 | 22 | #include <linux/uaccess.h> |
| 23 | 23 | #include <linux/inet.h> |
| 24 | −#include <linux/idr.h> | |
| 25 | 24 | #include <linux/file.h> |
| 26 | 25 | #include <linux/parser.h> |
| 27 | 26 | #include <linux/semaphore.h> |
| @@ -507,7 +506,7 @@ dont_need_post_recv: · 479 unmodified lines | ||
| 507 | 506 | * because doing if after could erase the REQ_STATUS_RCVD |
| 508 | 507 | * status in case of a very fast reply. |
| 509 | 508 | */ |
| 510 | − req->status = REQ_STATUS_SENT; | |
| 509 | + WRITE_ONCE(req->status, REQ_STATUS_SENT); | |
| 511 | 510 | err = ib_post_send(rdma->qp, &wr, NULL); |
| 512 | 511 | if (err) |
| 513 | 512 | goto send_error; |
| @@ -517,7 +516,7 @@ dont_need_post_recv: · 3 unmodified lines | ||
| 517 | 516 | |
| 518 | 517 | /* Handle errors that happened during or while preparing the send: */ |
| 519 | 518 | send_error: |
| 520 | − req->status = REQ_STATUS_ERROR; | |
| 519 | + WRITE_ONCE(req->status, REQ_STATUS_ERROR); | |
| 521 | 520 | kfree(c); |
| 522 | 521 | p9_debug(P9_DEBUG_ERROR, "Error %d in rdma_request()\n", err); |
| 523 | 522 | |
| @@ -22,7 +22,6 @@ · 21 unmodified lines | ||
| 22 | 22 | #include <linux/un.h> |
| 23 | 23 | #include <linux/uaccess.h> |
| 24 | 24 | #include <linux/inet.h> |
| 25 | −#include <linux/idr.h> | |
| 26 | 25 | #include <linux/file.h> |
| 27 | 26 | #include <linux/highmem.h> |
| 28 | 27 | #include <linux/slab.h> |
| @@ -263,7 +262,7 @@ p9_virtio_request(struct p9_client *client, struct p9_req_t *req) · 234 unmodified lines | ||
| 263 | 262 | |
| 264 | 263 | p9_debug(P9_DEBUG_TRANS, "9p debug: virtio request\n"); |
| 265 | 264 | |
| 266 | − req->status = REQ_STATUS_SENT; | |
| 265 | + WRITE_ONCE(req->status, REQ_STATUS_SENT); | |
| 267 | 266 | req_retry: |
| 268 | 267 | spin_lock_irqsave(&chan->lock, flags); |
| 269 | 268 | |
| @@ -469,7 +468,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req, · 199 unmodified lines | ||
| 469 | 468 | inlen = n; |
| 470 | 469 | } |
| 471 | 470 | } |
| 472 | − req->status = REQ_STATUS_SENT; | |
| 471 | + WRITE_ONCE(req->status, REQ_STATUS_SENT); | |
| 473 | 472 | req_retry_pinned: |
| 474 | 473 | spin_lock_irqsave(&chan->lock, flags); |
| 475 | 474 | |
| @@ -532,9 +531,10 @@ req_retry_pinned: · 56 unmodified lines | ||
| 532 | 531 | spin_unlock_irqrestore(&chan->lock, flags); |
| 533 | 532 | kicked = 1; |
| 534 | 533 | p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n"); |
| 535 | − err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD); | |
| 534 | + err = wait_event_killable(req->wq, | |
| 535 | + READ_ONCE(req->status) >= REQ_STATUS_RCVD); | |
| 536 | 536 | // RERROR needs reply (== error string) in static data |
| 537 | − if (req->status == REQ_STATUS_RCVD && | |
| 537 | + if (READ_ONCE(req->status) == REQ_STATUS_RCVD && | |
| 538 | 538 | unlikely(req->rc.sdata[4] == P9_RERROR)) |
| 539 | 539 | handle_rerror(req, in_hdr_len, offs, in_pages); |
| 540 | 540 | |
| @@ -157,7 +157,7 @@ again: · 156 unmodified lines | ||
| 157 | 157 | &masked_prod, masked_cons, |
| 158 | 158 | XEN_9PFS_RING_SIZE(ring)); |
| 159 | 159 | |
| 160 | − p9_req->status = REQ_STATUS_SENT; | |
| 160 | + WRITE_ONCE(p9_req->status, REQ_STATUS_SENT); | |
| 161 | 161 | virt_wmb(); /* write ring before updating pointer */ |
| 162 | 162 | prod += size; |
| 163 | 163 | ring->intf->out_prod = prod; |
| @@ -212,11 +212,13 @@ static void p9_xen_response(struct work_struct *work) · 48 unmodified lines | ||
| 212 | 212 | dev_warn(&priv->dev->dev, |
| 213 | 213 | "requested packet size too big: %d for tag %d with capacity %zd\n", |
| 214 | 214 | h.size, h.tag, req->rc.capacity); |
| 215 | − req->status = REQ_STATUS_ERROR; | |
| 215 | + WRITE_ONCE(req->status, REQ_STATUS_ERROR); | |
| 216 | 216 | goto recv_error; |
| 217 | 217 | } |
| 218 | 218 | |
| 219 | − memcpy(&req->rc, &h, sizeof(h)); | |
| 219 | + req->rc.size = h.size; | |
| 220 | + req->rc.id = h.id; | |
| 221 | + req->rc.tag = h.tag; | |
| 220 | 222 | req->rc.offset = 0; |
| 221 | 223 | |
| 222 | 224 | masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE(ring)); |