[RHEL7,COMMIT] ploop: avoid crash if ploop request has uninitialized preq_ub

Submitted by Konstantin Khorenko on Aug. 30, 2018, 9:06 a.m.

Details

Message ID 201808300906.w7U96Hg1001500@finist_ce7.work
State New
Series "ploop: initialize preq_ub for zero ploop requests as well"
Headers show

Commit Message

Konstantin Khorenko Aug. 30, 2018, 9:06 a.m.
The commit is pushed to "branch-rh7-3.10.0-862.11.6.vz7.71.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.11.6.vz7.71.5
------>
commit 05b2b31928729ec75b751813402a735c9aeb3166
Author: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date:   Wed Aug 29 17:24:10 2018 +0300

    ploop: avoid crash if ploop request has uninitialized preq_ub
    
    If we ever forget to initialize ploop request exec beancounter (preq_ub),
    we'll get a crash like following:
    
     BUG: unable to handle kernel NULL pointer dereference at 0000000000000100
     IP: [<ffffffffc05ef198>] iolimit_virtinfo+0x38/0x410 [vziolimit]
     PGD 0
     Oops: 0000 [#1] SMP
     Modules linked in: <skipped>
     CPU: 1 PID: 8472 Comm: ploop34743 ve: 0 Kdump: loaded Not tainted 3.10.0-862.11.6.vz7.71.5 #1 71.5
     Hardware name: DEPO Computers To Be Filled By O.E.M./H77 Pro4/MVP, BIOS P1.50B 03/20/2013
     task: ffff96f7f973a120 ti: ffff96f7f0bf0000 task.ti: ffff96f7f0bf0000
     RIP: 0010:[<ffffffffc05ef198>]  [<ffffffffc05ef198>] iolimit_virtinfo+0x38/0x410 [vziolimit]
     RSP: 0018:ffff96f7f0bf3ae0  EFLAGS: 00010246
     RAX: 0000000000000000 RBX: ffffffffc05f15b0 RCX: 0000000000000000
     RDX: ffff96f7f973a120 RSI: 0000000000000000 RDI: ffffffffc05f15b0
     RBP: ffff96f7f0bf3b48 R08: ffffffffc05ef160 R09: ffff96f7fab86008
     R10: 0000000000000000 R11: ffff96f7fab86008 R12: 0000000000000000
     R13: ffff96f7f0bf3ba8 R14: 0000000000000000 R15: ffff96f7f0bf3ba8
     FS:  0000000000000000(0000) GS:ffff96f81f240000(0000) knlGS:0000000000000000
     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
     CR2: 0000000000000100 CR3: 000000038c20e000 CR4: 00000000001607e0
     Call Trace:
      [<ffffffff95011b08>] ? kmem_cache_alloc+0xf8/0x240
      [<ffffffff94eae3cc>] do_virtinfo_notifier_call+0x4c/0x60
      [<ffffffff94eae420>] virtinfo_notifier_call+0x40/0x70
      [<ffffffff95142282>] submit_bio+0xf2/0x1c0
      [<ffffffffc03f4ff0>] dio_io_page+0x1b0/0x300 [pio_direct]
      [<ffffffffc03f51cc>] dio_read_page+0x1c/0x20 [pio_direct]
      [<ffffffffc03fe05d>] ploop1_read_index+0x1d/0x20 [pfmt_ploop1]
      [<ffffffffc05ad720>] ploop_read_map+0x1b0/0x2a0 [ploop]
      [<ffffffffc05ae88b>] ploop_find_map+0x6b/0x160 [ploop]
      [<ffffffffc05a7e36>] ploop_entry_request+0x6a6/0x15b0 [ploop]
      [<ffffffff94ec0486>] ? finish_wait+0x56/0x70
      [<ffffffffc05a971f>] ploop_req_state_process+0x9df/0xd10 [ploop]
      [<ffffffff94ec0800>] ? wake_up_atomic_t+0x30/0x30
      [<ffffffffc05a9c8d>] ploop_thread+0x23d/0x4f0 [ploop]
      [<ffffffffc05a9a50>] ? ploop_req_state_process+0xd10/0xd10 [ploop]
      [<ffffffff94ebf621>] kthread+0xd1/0xe0
      [<ffffffff94ebf550>] ? create_kthread+0x60/0x60
      [<ffffffff95553677>] ret_from_fork_nospec_begin+0x21/0x21
      [<ffffffff94ebf550>] ? create_kthread+0x60/0x60
     Code: d7 65 48 8b 14 25 c0 0e 01 00 41 56 41 55 41 54 53 48 83 ec 40 4c 8b b2 18 0b 00 00 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 <49> 8b 9e 00 01 00 00 48 85 db 0f 84 c8 02 00 00 8b 03 85 c0 74
     RIP  [<ffffffffc05ef198>] iolimit_virtinfo+0x38/0x410 [vziolimit]
      RSP <ffff96f7f0bf3ae0>
     CR2: 0000000000000100
    
    The crash happens because ploop_req_state_process() unconditionally sets
    current exec_ub to saved value in preq->preq_ub, even is it's NULL.
    
    But iolimit_virtinfo() which is called later does not expect current
    exec_ub is NULL and crashes.
    
    Let's add a failsafe check for preq->preq_ub != NULL and issue a warning
    otherwise.
    
    https://jira.sw.ru/browse/PSBM-88112
    
    Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
    Acked-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 drivers/block/ploop/dev.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index aeb2ad1cf0cf..2d53c8a5e2ac 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -2535,7 +2535,7 @@  static void ploop_req_state_process(struct ploop_request * preq)
 	struct io_context * saved_ioc = NULL;
 	int release_ioc = 0;
 #ifdef CONFIG_BEANCOUNTERS
-	struct user_beancounter *saved_ub;
+	struct user_beancounter *saved_ub = NULL;
 #endif
 
 	trace_req_state_process(preq);
@@ -2547,8 +2547,13 @@  static void ploop_req_state_process(struct ploop_request * preq)
 		release_ioc = 1;
 	}
 #ifdef CONFIG_BEANCOUNTERS
-	get_beancounter(preq->preq_ub);
-	saved_ub = set_exec_ub(preq->preq_ub);
+	WARN_ONCE(!preq->preq_ub,
+		  "preq_ub=NULL state=0x%lx eng_state=0x%lx error=0x%x\n",
+		  preq->state, preq->eng_state, preq->error);
+	if (preq->preq_ub) {
+		get_beancounter(preq->preq_ub);
+		saved_ub = set_exec_ub(preq->preq_ub);
+	}
 #endif
 
 	if (preq->eng_state != PLOOP_E_COMPLETE &&
@@ -2610,8 +2615,10 @@  static void ploop_req_state_process(struct ploop_request * preq)
 			preq->error = -EOPNOTSUPP;
 			ploop_complete_io_state(preq);
 #ifdef CONFIG_BEANCOUNTERS
-			saved_ub = set_exec_ub(saved_ub);
-			put_beancounter(saved_ub);
+			if (saved_ub) {
+				saved_ub = set_exec_ub(saved_ub);
+				put_beancounter(saved_ub);
+			}
 #endif
 			return;
 		}
@@ -2891,8 +2898,10 @@  static void ploop_req_state_process(struct ploop_request * preq)
 		put_io_context(ioc);
 	}
 #ifdef CONFIG_BEANCOUNTERS
-	saved_ub = set_exec_ub(saved_ub);
-	put_beancounter(saved_ub);
+	if (saved_ub) {
+		saved_ub = set_exec_ub(saved_ub);
+		put_beancounter(saved_ub);
+	}
 #endif
 }