Re: [net v2] net: wwan: t7xx: Fix FSM command timeout issue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sergey Ryazanov <ryazanov.s.a@xxxxxxxxx>

Fixes: d785ed945de6 ("net: wwan: t7xx: PCIe reset rescan")

The completion waiting was introduced in a different commit. I believe, the fix tag should be 13e920d93e37 ("net: wwan: t7xx: Add core components")


Got it.

[...]
  	if (cmd->flag & FSM_CMD_FLAG_WAIT_FOR_COMPLETION) {
  		*cmd->ret = result;

The memory for the result storage is allocated on the stack as well. And writing it unconditionally can cause unexpected consequences.


Got it.

[...]
  		wait_ret = wait_for_completion_timeout(&done,
  						       msecs_to_jiffies(FSM_CMD_TIMEOUT_MS));
-		if (!wait_ret)
+		if (!wait_ret) {
+			cmd->done = NULL;

We cannot access the command memory here, since fsm_finish_command() could release it already.


Got it.

[...]
Here we have an ownership transfer problem and a driver author has tried to solve it, but as noticed, we are still experiencing issues in case of timeout.

The command completion routine should not release the command memory unconditionally. Looks like the references counting approach should help us here. E.g.
1. grab a reference before we put a command into the queue
1.1. grab an extra reference if we are going to wait the completion
2. release the reference as soon as we are done with the command execution
3. in case of completion waiting release the reference as soon as we are done with waiting due to completion or timeout

Could you try the following patch? Please note, besides the reference counter introduction it also moves completion and result storage inside the command structure as advised by the completion documentation.


Yes, please let me try the following patch.
[...]

Thanks.

Jinjian,
Best Regards.




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux