[Devel] [PATCH vz7 v3] ploop: avoid crash if ploop request has uninitialized preq_ub

[Konstantin Khorenko]

From: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

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 at virtuozzo.com>
Acked-by: Konstantin Khorenko <khorenko at virtuozzo.com>

--
v2 changes:
   warning added if preq_ub == NULL found
v3 changes:
   use proper "%lx" to print long int

---
 drivers/block/ploop/dev.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

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
 }
 
-- 
2.15.1