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
