linux-kernel - Re: [PATCH] tools/counter: Add checking directory exists for make clean

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]

Message-ID: <e62f481d-f939-a12b-7f8a-ec1db95c7b8e@gmail.com>
Date:   Fri, 14 Jul 2023 20:57:21 +0700
From:   Anh Tuan Phan <tuananhlfc@...il.com>
To:     William Breathitt Gray <william.gray@...aro.org>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org, rongtao@...tc.cn,
        ricardo@...dini.net, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tools/counter: Add checking directory exists for make
 clean

Hi William, the race condition possibility is a good point. Thanks for
suggesting me. Will send a new version with using `rm -df` instead and
add the appropriate CC lists. Thank you!

On 7/10/23 08:07, William Breathitt Gray wrote:
> On Fri, Jul 07, 2023 at 09:08:35PM +0700, Anh Tuan Phan wrote:
>> rmdir requires the directory exist so it causes "make -C tools clean"
>> failed if someone only builds other tools but not counter. This commit
>> adds checking the directory exist before doing rmdir.
>>
>> Signed-off-by: Anh Tuan Phan <tuananhlfc@...il.com>
>> ---
>>  tools/counter/Makefile | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/counter/Makefile b/tools/counter/Makefile
>> index a0f4cab71fe5..2907f3b3094b 100644
>> --- a/tools/counter/Makefile
>> +++ b/tools/counter/Makefile
>> @@ -40,7 +40,9 @@ $(OUTPUT)counter_example: $(COUNTER_EXAMPLE)
>>  clean:
>>  	rm -f $(ALL_PROGRAMS)
>>  	rm -rf $(OUTPUT)include/linux/counter.h
>> -	rmdir -p $(OUTPUT)include/linux
>> +	@if [ -d $(OUTPUT)include/linux ]; then \
>> +		rmdir -p $(OUTPUT)include/linux; \
>> +	fi
>>  	find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete
>>
>>  install: $(ALL_PROGRAMS)
>> -- 
>> 2.34.1
> 
> Hi Anh,
> 
> Please CC <linux-iio@...r.kernel.org> and <linux-kernel@...r.kernel.org>
> as well in the future so Counter users and developers can become aware
> of relevant patches.
> 
> One worry I have with this approach is the possible race condition where
> the check for existence succeeds but the directory is deleted by another
> agent before our rmdir executes. However, I'm not sure how we could
> achieve such behavior atomically to prevent the issue.
> 
> One alternative I've considered is perhaps a single find command to
> search for and delete empty directories:
> 
>     find $(or $(OUTPUT),.) -type d -empty -delete
> 
> But this will delete directories not created by the makefile which I
> consider an unexpected behavior for the user (or at least very rude of
> the script to do).
> 
> Perhaps we should delete the directory tree explicitly:
> 
>     rm -df $(OUTPUT)include/linux
>     rm -df $(OUTPUT)include
> 
> Although we lose the symmetry of rmdir to our previous mkdir, this
> should prevent the race condition issue and succeed whether the
> directories still exist or not.
> 
> William Breathitt Gray