Received-SPF: pass (google.com: domain of linux-kernel+bounces-157029-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <fdc7ada8-6ca9-4835-982a-3e45ee7ac009@buaa.edu.cn>
Date: Wed, 24 Apr 2024 22:00:49 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] nfc: pn533: Fix null-ptr-deref in pn533_recv_frame()
To: Krzysztof Kozlowski <krzk@kernel.org>, Yuxuan Hu <20373622@buaa.edu.cn>,
 gregkh@linuxfoundation.org, johan@kernel.org, davem@davemloft.net,
 u.kleine-koenig@pengutronix.de, Jonathan.Cameron@huawei.com,
 francesco.dolcini@toradex.com, jirislaby@kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
 baijiaju1990@gmail.com, sy2239101@buaa.edu.cn
References: <f229256f-ceac-4a82-afa4-ecea67712ec3@kernel.org>
From: Yuxuan-Hu <yuxuanhu@buaa.edu.cn>
In-Reply-To: <f229256f-ceac-4a82-afa4-ecea67712ec3@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi Krzysztof,
Thanks for your reply.

On 2024/4/24 13:36, Krzysztof Kozlowski wrote:
> A bit better solution would be to NULL-ify dev->cmd at the beginning of
> pn533_send_async_complete(), because that seems logical. The complete
> callback takes ownership of dev->cmd, so why it performs the assignment
> at the end?
>
> However even above code will keep the race open for short period.
> Probably some locking would solve it or checking for dev->cmd in few
> places with barriers.
>
> Best regards,
> Krzysztof

I think adding a lock seems to be a better solution, however, acquire a 
lock on each access to dev->cmd does not seem to be an appropriate 
implementation.
I wonder whether you think it is appropriate to acquire a lock at the 
beginning of pn533_recv_frame(), and release it when 
pn533_wq_cmd_complete() is finished, thus ensuring that another work 
will not start when one pn533_wq_cmd_complete() work is not yet finished.

Best regards,
Yuxuan