[U-Boot] [PATCH] binman: ensure temp filenames don't collide

[Stephen Warren]

On 07/19/2018 08:17 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On 19 July 2018 at 15:14, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 07/18/2018 07:32 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On 16 July 2018 at 16:51, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> The U-Boot Makefile can invoke binman multiple times in parallel. This
>>>> is problematic because binman uses a static hard-coded temporary file
>>>> name. If two instances of binman use that filename at the same time, one
>>>> writing one reading, they may silently read the wrong content or actively
>>>> detect missing signatures and error out the build process. Fix this by
>>>> using a PID-specific filename instead.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>> ---
>>>>  tools/binman/control.py | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/binman/control.py b/tools/binman/control.py
>>>> index a40b300fdacb..515999278949 100644
>>>> --- a/tools/binman/control.py
>>>> +++ b/tools/binman/control.py
>>>> @@ -121,7 +121,7 @@ def Binman(options, args):
>>>>              # output into a file in our output directly. Then scan it for use
>>>>              # in binman.
>>>>              dtb_fname = fdt_util.EnsureCompiled(dtb_fname)
>>>> -            fname = tools.GetOutputFilename('u-boot-out.dtb')
>>>> +            fname = tools.GetOutputFilename('u-boot-out.dtb') + str(os.getpid())
>>>>              with open(dtb_fname) as infd:
>>>>                  with open(fname, 'wb') as outfd:
>>>>                      outfd.write(infd.read())
>>>> --
>>>> 2.18.0
>>>>
>>>
>>> But the output directory is itself (normally) a temporary dir. That
>>> determines the directly which GetOutputFilename() uses. So I am not
>>> sure how this can happen in practice?
>>
>> IIRC, the output directory for all 3 files was the same; the root of the
>> build output directory. Perhaps the fact I run "make O=build-xxx" rather
>> than just "make" makes a difference?
> 
> Yes you are right - the -O flag sets the output directory and it
> no-longer uses a temporary directory.
> 
> But if we add a PID to every filename then we end up with multiple
> output files and we don't know which is right.
> 
> I think the correct fix is to change the Makefile to only run binman
> once. It makes no sense to run it multiple times.

IIRC, this filename is just a temp file that eventually gets removed
after binman runs. Certainly there are not *.${pid} files left around
after the build has completed.