lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 10 Nov 2023 11:38:42 +0800
From:   Qiang Yu <quic_qianyu@...cinc.com>
To:     Manivannan Sadhasivam <mani@...nel.org>
CC:     <quic_jhugo@...cinc.com>, <mhi@...ts.linux.dev>,
        <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <quic_cang@...cinc.com>, <quic_mrana@...cinc.com>
Subject: Re: [PATCH v3 3/4] bus: mhi: host: Avoid processing buffer and event
 of a disable channel


On 11/10/2023 12:32 AM, Manivannan Sadhasivam wrote:
> On Tue, Nov 07, 2023 at 03:16:04PM +0800, Qiang Yu wrote:
>> Ckeck mhi channel state after getting chan->lock to ensure that we only
>> queue buffer to an enabled channel and process event of an enabled channel.
>>
> This commit message doesn't give proper explanation on how the channel can go to
> disabled state in between parse_xfer_event() and mhi_gen_tre().
>
> - Mani

Hi Mani. How about following commit message

MHI channel state is protected by mhi_chan->lock. Hence, after core drops
mhi_chan->lock during processing xfer event, it can not prevent channel
state being changed if client closes channel or driver is removed at this
time. So let's check mhi channel state after getting chan->lock again to 
avoid
queuing buffer to a disabled channel in xfer callback and stop processing
event of the disabled channel.

>> Signed-off-by: Qiang Yu <quic_qianyu@...cinc.com>
>> ---
>>   drivers/bus/mhi/host/main.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
>> index a236dc2..b137d54 100644
>> --- a/drivers/bus/mhi/host/main.c
>> +++ b/drivers/bus/mhi/host/main.c
>> @@ -672,6 +672,8 @@ static int parse_xfer_event(struct mhi_controller *mhi_cntrl,
>>   			}
>>   
>>   			read_lock_bh(&mhi_chan->lock);
>> +			if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED)
>> +				goto end_process_tx_event;
>>   		}
>>   		break;
>>   	} /* CC_EOT */
>> @@ -1211,6 +1213,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
>>   
>>   	/* Protect accesses for reading and incrementing WP */
>>   	write_lock_bh(&mhi_chan->lock);
>> +	if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED)
>> +		return -EINVAL;
>>   
>>   	buf_ring = &mhi_chan->buf_ring;
>>   	tre_ring = &mhi_chan->tre_ring;
>> -- 
>> 2.7.4
>>
>>

Powered by blists - more mailing lists