netdev - Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)

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: <20190830064722.GJ15257@kroah.com>
Date:   Fri, 30 Aug 2019 08:47:22 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Julia Kartseva <hex@...com>, ast@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>, rdna@...com,
        bpf@...r.kernel.org, daniel@...earbox.net, netdev@...r.kernel.org,
        kernel-team@...com
Subject: Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add
 libbpf_prog_type_(from|to)_str helpers

On Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov wrote:
> On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
> > > On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> > > > 
> > > > Greg, Thomas, libbpf is extracted from the kernel sources and
> > > > maintained in a clone repo on GitHub for ease of packaging.
> > > > 
> > > > IIUC Alexei's concern is that since we are moving the commits from
> > > > the kernel repo to the GitHub one we have to preserve the commits
> > > > exactly as they are, otherwise SOB lines lose their power.
> > > > 
> > > > Can you provide some guidance on whether that's a valid concern, 
> > > > or whether it's perfectly fine to apply a partial patch?
> > > 
> > > Right. That's exactly the concern.
> > > 
> > > Greg, Thomas,
> > > could you please put your legal hat on and clarify the following.
> > > Say some developer does a patch that modifies
> > > include/uapi/linux/bpf.h
> > > ..some other kernel code...and
> > > tools/include/uapi/linux/bpf.h
> > > 
> > > That tools/include/uapi/linux/bpf.h is used by perf and by libbpf.
> > > We have automatic mirror of tools/libbpf into github/libbpf/
> > > so that external projects and can do git submodule of it,
> > > can build packages out of it, etc.
> > > 
> > > The question is whether it's ok to split tools/* part out of
> > > original commit, keep Author and SOB, create new commit out of it,
> > > and automatically push that auto-generated commit into github mirror.
> > 
> > Note, I am not a laywer, and am not _your_ lawyer either, only _your_
> > lawyer can answer questions as to what is best for you.
> > 
> > That being said, from a "are you keeping the correct authorship info",
> > yes, it sounds like you are doing the correct thing here.
> > 
> > Look at what I do for stable kernels, I take the original commit and add
> > it to "another tree" keeping the original author and s-o-b chain intact,
> > and adding a "this is the original git commit id" type message to the
> > changelog text so that people can link it back to the original.
> 
> I think you're describing 'git cherry-pick -x'.

Well, my scripts don't use git, but yes, it's much the same thing :)

> The question was about taking pieces of the original commit. Not the whole commit.
> Author field obviously stays, but SOB is questionable.

sob matters to the file the commit is touching, and if it is identical
to the original file (including same license), then it should be fine.

> If author meant to change X and Y and Z. Silently taking only Z chunk of the diff
> doesn't quite seem right.

It can be confusing, I agree.

> If we document that such commit split happens in Documentation/bpf/bpf_devel_QA.rst
> do you think it will be enough to properly inform developers?
> The main concern is the surprise factor when people start seeing their commits
> in the mirror, but not their full commits.

Personally, I wouldn't care, but maybe you should just enforce the fact
that the original patch should ONLY touch Z, and not X and Y in the same
patch, to make this a lot more obvious.

Patches should only be doing "one logical thing" in the first place, but
maybe you also need to touch other things when doing a change that you
can't do this, I really do not know, sorry.

greg k-h