[PATCH linux dev-4.10 2/3] drivers: fsi: sbefifo: Fix memory leak

Eddie James eajames at linux.vnet.ibm.com
Thu Oct 19 00:47:52 AEDT 2017



On 10/18/2017 08:24 AM, Andrew Jeffery wrote:
> On Tue, 2017-10-17 at 16:35 -0500, Eddie James wrote:
>> From: "Edward A. James" <eajames at us.ibm.com>
>>   
>> Transfers weren't being cleaned up if they were complete but the user
>> never finished reading. This is very common.
> I'm curious as to why this is common, but regardless, the patch looks
> fine given the premise.

Users just seemed to do this very frequently, from my tracing. They just 
don't finish reading all the available data and instead just close the 
client. That's because the actual data packet is some number of words 
less than the amount of data read and returned by the SBEFIFO. In 
practice, a user should read all the extra bytes to check them to make 
sure the transfer was successful, but it's actually difficult to know 
how much extra data is there.

This only happens if the driver has actually read all the data out of 
the FIFO and therefore removed the transfer from it's queue.

Thanks,
Eddie

>
> Andrew
>
>>   
>> Signed-off-by: Edward A. James <eajames at us.ibm.com>
>> ---
>>   drivers/fsi/fsi-sbefifo.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>   
>> diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
>> index c914eaf..764d8f3 100644
>> --- a/drivers/fsi/fsi-sbefifo.c
>> +++ b/drivers/fsi/fsi-sbefifo.c
>> @@ -321,6 +321,11 @@ static void sbefifo_client_release(struct kref *kref)
>>   
>>   	if (!READ_ONCE(sbefifo->rc)) {
>>   		list_for_each_entry(xfr, &client->xfrs, client) {
>> +			if (test_bit(SBEFIFO_XFR_COMPLETE, &xfr->flags)) {
>> +				kfree(xfr);
>> +				continue;
>> +			}
>> +
>>   			/*
>>   			 * The client left with pending or running xfrs.
>>   			 * Cancel them.



More information about the openbmc mailing list