Re: [bug report] block: avoid use-after-free on throttle data

[Ming Lei <ming.lei@redhat.com>]

Hello Dan,

On Tue, Mar 22, 2022 at 09:55:04AM +0300, Dan Carpenter wrote:
> Hello Ming Lei,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch ee37eddbfa9e: "block: avoid use-after-free on throttle 
> data" from Mar 18, 2022, leads to the following Smatch complaint:
> 
>     block/blk-throttle.c:1189 throtl_pending_timer_fn()
>     error: we previously assumed 'tg' could be null (see line 1147)
> 
> block/blk-throttle.c
>   1146		/* throtl_data may be gone, so figure out request queue by blkg */
>   1147		if (tg)
>                     ^^
> The patch adds a new check
> 
>   1148			q = tg->pd.blkg->q;
>   1149		else
>   1150			q = td->queue;
>   1151	
>   1152		spin_lock_irq(&q->queue_lock);
>   1153	
>   1154		if (!q->root_blkg)
>   1155			goto out_unlock;
>   1156	
>   1157		if (throtl_can_upgrade(td, NULL))
>   1158			throtl_upgrade_state(td);
>   1159	
>   1160	again:
>   1161		parent_sq = sq->parent_sq;
>   1162		dispatched = false;
>   1163	
>   1164		while (true) {
>   1165			throtl_log(sq, "dispatch nr_queued=%u read=%u write=%u",
>   1166				   sq->nr_queued[READ] + sq->nr_queued[WRITE],
>   1167				   sq->nr_queued[READ], sq->nr_queued[WRITE]);
>   1168	
>   1169			ret = throtl_select_dispatch(sq);
>   1170			if (ret) {
>   1171				throtl_log(sq, "bios disp=%u", ret);
>   1172				dispatched = true;
>   1173			}
>   1174	
>   1175			if (throtl_schedule_next_dispatch(sq, false))
>   1176				break;
>   1177	
>   1178			/* this dispatch windows is still open, relax and repeat */
>   1179			spin_unlock_irq(&q->queue_lock);
>   1180			cpu_relax();
>   1181			spin_lock_irq(&q->queue_lock);
>   1182		}
>   1183	
>   1184		if (!dispatched)
>   1185			goto out_unlock;
>   1186	
>   1187		if (parent_sq) {
>   1188			/* @parent_sq is another throl_grp, propagate dispatch */
>   1189			if (tg->flags & THROTL_TG_WAS_EMPTY) {
>                             ^^^^^^^^^
> But the old code dereferences "tg" without checking.

Here if 'parent_sq' isn't NULL, tg won't be NULL, see sq_to_tg()


Thanks,
Ming