From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 19 Jul 2018 22:43:49 -0600 Subject: [U-Boot] [PATCH] binman: ensure temp filenames don't collide In-Reply-To: References: <20180716225148.9136-1-swarren@wwwdotorg.org> <22be70bb-35f8-8cde-d377-d04e00991185@wwwdotorg.org> Message-ID: <6ab16c41-913f-4f96-e0c7-c2cdf756ba00@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 07/19/2018 08:17 PM, Simon Glass wrote: > Hi Stephen, > > On 19 July 2018 at 15:14, Stephen Warren wrote: >> On 07/18/2018 07:32 PM, Simon Glass wrote: >>> Hi Stephen, >>> >>> On 16 July 2018 at 16:51, Stephen Warren wrote: >>>> From: Stephen Warren >>>> >>>> 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 >>>> --- >>>> 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.