[PATCH 0/1] MR8270: mf: Prevent double submission of session commands.

[Yuxuan Shui (＠yshui)]

This is not the right fix, and I need some help. Let me explain.

`session_submit_command` checks that:

1. `session->commands` is empty, and
2. `SESSION_FLAG_PENDING_COMMAND` is NOT set

before submitting the `session_op`. The assumption is, if there are queued commands, or if there is a command currently running, that pending command will submit subsequent commands when it completes.

However, in this call chain:

```
#0  0x00006fffe1d27ad7 in create_async_result (object=object@entry=0x0, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0, out=out@entry=0x7e6242bac7a0) at ../dlls/rtworkq/queue.c:1229
#1  0x00006fffe1d222d5 in RtwqCreateAsyncResult (object=object@entry=0x0, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0, out=out@entry=0x7e6242bac7a0) at ../dlls/rtworkq/queue.c:1245
#2  0x00006fffd45e2096 in MFPutWorkItem (queue=queue@entry=1, callback=callback@entry=0x7e62489f1c08,
    state=state@entry=0x7e61cb5322b0) at ../dlls/mfplat/queue.c:46
#3  0x00006fffe41244ee in session_command_complete (session=0x7e62489f1be0) at ../dlls/mf/session.c:975
#4  0x00006fffe412d00f in session_reset (session=session@entry=0x7e62489f1be0) at ../dlls/mf/session.c:1051
#5  0x00006fffe41299fc in session_handle_source_shutdown (session=session@entry=0x7e62489f1be0)
    at ../dlls/mf/session.c:2957
#6  0x00006fffe4126e90 in session_events_callback_Invoke (iface=0x7e62489f1c20, result=<optimized out>)
    at ../dlls/mf/session.c:4445
```

nowhere was the `SESSION_FLAG_PENDING_COMMAND` flag set. In addition to that, `session_events_callback_Invoke` is not a command. So this could happend:

```
Thread 1                                 | Thread 2
call session_events_callback_Invoke      |
                                         | call session_submit_command
                                         | <critical section>
                                         | list_empty(&session->commands) is true, and PENDING_COMMAND not set
                                         | MFPutWorkItem(op)
                                         | list_add(&session->commands, op) <- (1)
                                         | </critical_section>
call session_handle_source_shutdown      |
<critical_section>                       |
call session_reset                       |
call session_command_complete            |
list_head(&session->commands)            |
op is returned because of (1)            |
MFPutWorkItem(op) again                  |
</critical_section>                      |
```

The result is this op will be executed twice, and also double freed, because `session_commands_callback_Invoke` releases the `op`.

* * *

There is even another way an `op` can be submitted twice, `session_handle_start_error` calls `session_reset`, which already calls `session_command_complete`, and later it calls `session_command_complete_with_event` again. I think a missing return?

[PATCH 1/1] mf: Prevent double submission of session commands.

[Yuxuan Shui]

From: Yuxuan Shui

---
 dlls/mf/session.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/dlls/mf/session.c b/dlls/mf/session.c
index 483ea6f904f..5ea93da3cee 100644
--- a/dlls/mf/session.c
+++ b/dlls/mf/session.c
@@ -972,6 +972,11 @@ static void session_command_complete(struct media_session *session)
     if ((e = list_head(&session->commands)))
     {
         op = LIST_ENTRY(e, struct session_op, entry);
+        if (op->submitted)
+        {
+            ERR("op %p already submitted\n", op);
+            return;
+        }
         hr = MFPutWorkItem(MFASYNC_CALLBACK_QUEUE_STANDARD, &session->commands_callback, &op->IUnknown_iface);
         op->submitted = SUCCEEDED(hr);
     }

[PATCH 0/1] MR8270: Draft: mf: Prevent double submission of session commands.

[Yuxuan Shui (＠yshui)]

On second thought, this doesn't need a race to happen. `session_events_callback_Invoke` is not a command so it shouldn't call `session_command_complete` at all I think?