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

[Simon Glass]

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.

Regards,
Simon