@samitouri / QOSamiQemu / commits / 5297a0fc65

lsi53c895a: fix use-after-free of cancelled request

When processing the Message Out phase, the lsi53c895a controller can cancel a request and the continue by processing more messages. When this happens, it is important that a cancelled request is not processed further, because scsi_req_cancel can cause the request to be freed. Right now this is happening in two cases, but not when cancelling the entire queue of requests after an ABORT, CLEAR QUEUE or BUS DEVICE RESET message. In that case, a subsequent ABORT TAG message can use a dangling current_req. There are three possible fixes: - add a missing check inside the loop, clearing current_req if p->req == current_req. This is obvious but complicates the code inside the foreach loop. - change the conditional prior to the loop from "if (s->current)" to "if (current_req)". This would work, because s->current != NULL implies current_req != NULL, and would clear current_req correctly. However it is less obvious because the point of the code is to clear the entire queue, which consists of s->current and s->queue; current_req is not special here. - delay the retrieval of current_req until an ABORT TAG message is seen. This is the most correct option, because the SCSI protocol only deals with tags; requests are a QEMU concept that only makes sense for the purpose of calling into the SCSI layer. Reported-by: Wei Che Kao <skps96g313.cs10@gmail.com> Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo Bonzini committed May 15, 2026 at 11:01 UTC 5297a0fc65317ba7f79ef44ce7a44e41d15fdb27
1 file changed +5 -4
hw/scsi/lsi53c895a.c
+5 -4
@@ -1000,10 +1000,8 @@ static void lsi_do_msgout(LSIState *s)
1000
1001 if (s->current) {
1002 current_tag = s->current->tag;
1003 - current_req = s->current;
1003 } else {
1004 current_tag = s->select_tag;
1006 - current_req = lsi_find_by_tag(s, current_tag);
1005 }
1006
1007 trace_lsi_do_msgout(s->dbc);
@@ -1058,9 +1056,13 @@ static void lsi_do_msgout(LSIState *s)
1056 case 0x0d:
1057 /* The ABORT TAG message clears the current I/O process only. */
1058 trace_lsi_do_msgout_abort(current_tag);
1059 + if (s->current) {
1060 + current_req = s->current;
1061 + } else {
1062 + current_req = lsi_find_by_tag(s, current_tag);
1063 + }
1064 if (current_req && current_req->req) {
1065 scsi_req_cancel(current_req->req);
1063 - current_req = NULL;
1066 }
1067 lsi_disconnect(s);
1068 break;
@@ -1086,7 +1088,6 @@ static void lsi_do_msgout(LSIState *s)
1088 /* clear the current I/O process */
1089 if (s->current) {
1090 scsi_req_cancel(s->current->req);
1089 - current_req = NULL;
1091 }
1092
1093 /* As the current implemented devices scsi_disk and scsi_generic