Re: Sourceware mitigating and preventing the next xz-backdoor

Alejandro Colomar <[email protected]> Sun, 21 Apr 2024 22:52:11 +0200
Newsgroups	gmane.comp.gnu.binutils,gmane.comp.gcc.devel,gmane.comp.gdb.devel,gmane.comp.lib.glibc.alpha
Message-ID	<ZiV8e8Xm4GFGbQ2E@debian>
On Sun, Apr 21, 2024 at 10:40:14PM +0200, Alejandro Colomar wrote:
> On Sun, Apr 21, 2024 at 05:30:52PM +0200, Mark Wielaard wrote:
> > Hi Alejandro,
> 
> Hi Mark,
> 
> I've reordered your message, to organize my response.
> 
> > On Wed, Apr 10, 2024 at 06:30:42PM +0200, Alejandro Colomar wrote:
> > > It would also be interesting to require showing range-diffs between
> > > patch revisions.  They make it much more difficult to introduce a
> > > vulnerability after a reviewer has turned its mins into approving the
> > > patch.  Of course, the patch could go in if the submitter lies in the
> > > range-diff and the vuln is undetected, but then it can be verified a
> > > posteriory to prove that there was a lie.
> > 
> > Could you give an example of using git range-diff?
> 
> Sure!
> 
> > Normally when asked for changes to a
> > patch (series) I do an git rebase -i (on the local branch I used to
> > develop the feature/bug fix) and split/commit all requested changes
> > and then sent the new patches with git send-email again.
> 
> I do the same thing.
> 
> > But I guess
> > to use/combine that with git range-diffs I should start creating new
> > local branches for each patch (series) in development?
> 
> No; it's compatible with that workflow.
> 
> > How do you go from
> > v1 of a patch (series) to a v2?
> 
> I'll start first with how you go from nothing to v1, which will help
> explain how you go from v1 to v2.
> 
> Let's say I have a branch 'unifont' for adding the Unifont font to the
> Linux man-pages' PDF book.  I have it in a branch that applies on top of
> 'master'.
> 
> 	$ git log --oneline --graph master..unifont;
> 	* 892a12470 (HEAD -> unifont, alx/contrib, contrib) share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> 	* d80376b08 share/mk/: build-pdf-book: Use Unifont
> 	* 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> 
> I want to send that as a patch set (v1, of course, since it's the first
> send).  I would already use range-diff when generating the patches:
> 
> 	$ git format-patch -o ./patches master..HEAD \
> 		--range-diff=master -v1 --cover-letter;
> 	./patches/v1-0000-cover-letter.patch
> 	./patches/v1-0001-share-mk-build-fonts-unifont-Build-UnifontR-from-.patch
> 	./patches/v1-0002-share-mk-build-pdf-book-Use-Unifont.patch
> 	./patches/v1-0003-share-mk-build-fonts-unifont-Specify-spacewidth-i.patch
> 
> Why would you do that in v1?  It notes the commit IDs of the patches, so
> you can later check them when preparing v2; we'll come back to that.
> For now, see what this adds to the patch set:
> 
> 	$ tail ./patches/v1-0000-cover-letter.patch ;
> 	 create mode 100644 share/mk/build/fonts/unifont/pfa.mk
> 	 create mode 100644 share/mk/configure/build-depends/fonts-unifont/unifont.otf.mk
> 
> 	Range-diff against v0:
> 	-:  --------- > 1:  7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> 	-:  --------- > 2:  d80376b08 share/mk/: build-pdf-book: Use Unifont
> 	-:  --------- > 3:  892a12470 share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> 	-- 
> 	2.43.0
> 
> You can see the IDs of the commits that form this patch set, which match
> the git-log(1) that I showed above.  Now someone suggests a change.  For
> this example, I wasn't happy with a commit message, and added a space to
> the third commit message subject line.  The git-log(1) is now:
> 
> 	$ git log --oneline --graph master..unifont
> 	* bc7fa7d92 (HEAD -> unifont) share/mk/: build-fonts-unifont: Specify space width in afmtodit(1)
> 	* d80376b08 share/mk/: build-pdf-book: Use Unifont
> 	* 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> 
> Let's generate a v2 patch set, showing the range-diff against v1.  We
> need to check the commit IDs of the first set, which can be found in the
> mailing list archives, thanks to the trick we used.  The v1 range was
> 7ec952012^..892a12470.  So we just pass that range:
> 
> 	$ git format-patch -o ./patches/ master..HEAD \
> 		--range-diff=7ec952012^..892a12470 -v2 --cover-letter;
> 	./patches/v2-0000-cover-letter.patch
> 	./patches/v2-0001-share-mk-build-fonts-unifont-Build-UnifontR-from-.patch
> 	./patches/v2-0002-share-mk-build-pdf-book-Use-Unifont.patch
> 	./patches/v2-0003-share-mk-build-fonts-unifont-Specify-space-width-.patch
> 
> The v2 cover letter shows the changes introduced since v1:
> 
> 	$ tail -n20 ./patches/v2-0000-cover-letter.patch 
> 	 create mode 100644 share/mk/build/fonts/unifont/dit.mk
> 	 create mode 100644 share/mk/build/fonts/unifont/pfa.mk
> 	 create mode 100644 share/mk/configure/build-depends/fonts-unifont/unifont.otf.mk
> 
> 	Range-diff against v1:
> 	1:  7ec952012 = 1:  7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> 	2:  d80376b08 = 2:  d80376b08 share/mk/: build-pdf-book: Use Unifont
> 	3:  892a12470 ! 3:  bc7fa7d92 share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> 	    @@ Metadata
> 	     Author: Alejandro Colomar <[email protected]>
> 	     
> 	      ## Commit message ##
> 	    -    share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> 	    +    share/mk/: build-fonts-unifont: Specify space width in afmtodit(1)
> 	     
> 		 Link: <https://lore.kernel.org/linux-man/ZiQ_mTQHPq3ig723@debian/T/#t>
> 		 Suggested-by: "G. Branden Robinson" <[email protected]>
> 	-- 
> 	2.43.0
> 
> If too much time passes between the last time the commit was rebased and
> when you create the range diff, you might need to use tags, such as
> 'unifont-v1', to avoid git(1) collecting garbagge and removing those
> old commits.  But if you send shortly after rebasing, you won't need
> that.
> 
> Lately, I haven't sent many patches to mailing lists, except for trivial
> oneliners, so I don't remember an example where I used this, but I can
> point to several github PRs where I've used this to document all changes
> that have happened to my patch sets, even when the maintainer has edited
> them, to be fully transparent.  For example:
> <https://github.com/neomutt/neomutt/pull/4221#issuecomment-2061341972>
> <https://github.com/neomutt/neomutt/pull/4221#issuecomment-2052533059>

And here's an example where the maintainer pushed a version different
from my last revision, because he rebased after applying other changes
to his branch, and I showed a last range diff to prove that he didn't
do anything weird with my patches:

<https://github.com/neomutt/neomutt/pull/4247#issuecomment-2061145881>

> 
> Have a lovely day!
> Alex
> 
> -- 
> <https://www.alejandro-colomar.es/>

-- 
<https://www.alejandro-colomar.es/>
signature.asc (application/pgp-signature, 833 B)