Page MenuHomeFreeBSD

armv8crypto: add some missing copyright statements
ClosedPublic

Authored by mhorne on Mar 15 2021, 2:03 PM.
Tags
None
Referenced Files
F103527962: D29268.diff
Tue, Nov 26, 3:25 AM
Unknown Object (File)
Sat, Nov 16, 9:44 PM
Unknown Object (File)
Mon, Nov 4, 12:30 PM
Unknown Object (File)
Mon, Nov 4, 12:30 PM
Unknown Object (File)
Mon, Nov 4, 12:30 PM
Unknown Object (File)
Mon, Nov 4, 12:29 PM
Unknown Object (File)
Mon, Nov 4, 12:28 PM
Unknown Object (File)
Mon, Nov 4, 12:04 PM
Subscribers

Details

Summary

The recently added AES-XTS bits were ported from the aesni(4) driver,
but without proper copyright attribution. I failed to check this when
committing that change.

Copyright is owed to pjd@ for the initial AES-XTS work added to aesni(4)
in ac970319ffdf1. This was based on some work in OpenBSD by Damian
Miller, whose copyright was added after-the-fact in 45b56a6ba2b75.

Later modifications to the aesni(4) XTS code were done by jmg@ in
ff6c7bf5caf8e.

Fixes: 4979620ece984 ("armv8crypto: add AES-XTS support")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Thank you.

This looks correct except for I'm not sure what the policy is on modifying that statement. Easiest likely would be to ask the FreeBSD foundation for guidance on this, as this is their statement that is being modfiied.

sys/crypto/armv8/armv8_crypto_wrap.c
6

someone who knows law would need to decide if this is valid to modify or not. I am not qualified to make that statement.

mhorne added inline comments.
sys/crypto/armv8/armv8_crypto_wrap.c
6

@andrew or @emaste - are you able to weigh in on this point? I will drop the tweak otherwise.

I've made my suggestion based on normal project policy for files that are copied and then substantially modified, as is the case here.

sys/crypto/armv8/armv8_crypto_wrap.c
2–4

I'm not convinced that this is a copyright violation of a magnitude to add all these holders here. They didn't contribute significant non-boilerplate portions of this file, so I worry that it would be misleading to have them here, but see below.

6

I'd not make this change. The vast majority of the copyright protectable parts of this file was developed by Andrew.

36

Instead, I'd add the following here like I've done when I've copied things in CAM, for example, acknowledging the copying

* This file derived from aesni_wrap.c:
* Copyright (C) 2008 Damien Miller <djm@mindrot.org>
* Copyright (c) 2010 Konstantin Belousov <kib@FreeBSD.org>
* Copyright (c) 2010-2011 Pawel Jakub Dawidek <pawel@dawidek.net>
* Copyright 2012-2013 John-Mark Gurney <jmg@FreeBSD.org>
* Copyright (c) 2014 The FreeBSD Foundation
sys/crypto/armv8/armv8_crypto_wrap.c
2–4

I should have said 'the magnitude of the material copied' since I'm not trying to make a judgement one way or other whether this rises to a copyright violation, which is a legal term of art no body on this review is licensed to proffer.

sys/crypto/armv8/armv8_crypto_wrap.c
36

Thank you for the advice, Warner. This seems like a sensible way to handle this type of case. I will update the review shortly.

Note file derivation and associated copyright at the end of the copyright statement.

the phrase "This file derived from" has no history in our tree, I'd prefer the phrase I proposed.

I'd prefer for the full path (or at least relative to sys) be included as not everyone knows our source tree well enough.

sys/crypto/armv8/armv8_crypto_wrap.c
30

I disagree w/ this statement. I would prefer:

Portions of this file are copied and modified from sys/crypto/aesni/aesni_wrap.c and subject to:

sys/crypto/armv8/armv8_crypto_wrap.c
30

The more standard wording in the tree is 'derived from' which is why I suggested that.

The wording "and subject to" concerns me. I don't think it means anything legally here and I'd rather not imply they are 'subject to' a copyright without there being an attendant license statement.
A quick grep shows that we have about two dozen instances that use 'derived from' and none with the 'copied and modified' in the tree today.
There's also a smattering of other phrases like 'inspired by' and the like that I could quickly locate.
In code we've removed there was also "This software is a derivative work of if_ed.c version 1.56 by David Greenman" followed by his copyright for a similar copy but replace that's happened here.

The original license in aesni_wrap.c just says that the copyright needed to be preserved along with the disclaimer. Since this file is licensed under the same license, both those terms are satisfied by my suggestion. We should, imho, use the wording we use elsewhere here because the more variations we have in wording means more work for people doing license audits.

sys/crypto/armv8/armv8_crypto_wrap.c
30

please include links to these examples of derived so I can compare wordings. I attempted to find derived from, but did not see any that resembled this wording.

The reason I included the subject to is that just because they are derived works does NOT remove their original copyright.

The originally the copyright was not preserved, which started this whole problem. If it had been retained like it was required, we wouldn't be here today.

Neither of us are lawyers, maybe you should get a lawyer to chime in on this, but IMO, the proposed derived from wording implied that the copyright does not apply any longer and I disagree w/ that.

sys/crypto/armv8/armv8_crypto_wrap.c
30

I used the following grep to find it:

% grep -r 'derived from' sys | grep ":.*\.[ch]"
sys/arm64/broadcom/genet/if_genetreg.h:/* derived from NetBSD's bcmgenetreg.h */
sys/cam/nvme/nvme_xpt.c: * derived from ata_xpt.c: Copyright (c) 2009 Alexander Motin <mav@FreeBSD.org>
sys/contrib/device-tree/include/dt-bindings/reset/altr,rst-mgr-s10.h: * derived from Steffen Trumtrar's "altr,rst-mgr-a10.h"
sys/contrib/openzfs/module/os/freebsd/zfs/crypto_os.c: * Portions of this file are derived from sys/geom/eli/g_eli_hmac.c
sys/contrib/openzfs/module/icp/algs/aes/aes_impl_generic.c: * This file is derived from the file  rijndael-alg-fst.c  taken from the
sys/xen/interface/io/tpmif.h: * This code has been derived from tools/libxc/xen/io/netif.h
sys/dev/exca/excavar.h: * This software may be derived from NetBSD i82365.c and other files with
sys/dev/exca/excareg.h: * This software may be derived from NetBSD i82365.c and other files with
sys/dev/exca/exca.c: * This software may be derived from NetBSD i82365.c and other files with
sys/dev/sound/pci/via8233.c: * Portions of this code derived from via82c686.c:
sys/dev/sound/usb/uaudio.c:/* The following table is derived from Linux's quirks-table.h */
sys/dev/pcf/pcf_isa.c: * derived from sys/i386/isa/pcf.c which is:
sys/dev/usb/misc/udbp.h: * This file was derived from src/sys/netgraph/ng_sample.h, revision 1.1
sys/dev/bhnd/cores/pmu/bhnd_pmureg.h: * This file is derived from the sbchipc.h header contributed by Broadcom
sys/dev/bhnd/cores/pmu/bhnd_pmu_private.h: * This file is derived from the hndpmu.h header contributed by Broadcom
sys/dev/bhnd/cores/pmu/bhnd_pmu_subr.c: * This file is derived from the hndpmu.c source contributed by Broadcom
sys/dev/bhnd/cores/pcie2/bhnd_pcie2_reg.h: * This file is derived from the pcie_core.h and pcie2_core.h headers
sys/dev/bhnd/cores/pci/bhnd_pcireg.h: * This file is derived from the hndsoc.h, pci_core.h, and pcie_core.h headers
sys/dev/bhnd/cores/chipc/pwrctl/bhnd_pwrctl_subr.c: * This file is derived from the siutils.c source distributed with the
sys/dev/bhnd/cores/chipc/pwrctl/bhnd_pwrctl.c: * Portions of this file were derived from the siutils.c source distributed with
sys/dev/bhnd/cores/chipc/chipcreg.h: * This file is derived from the sbchipc.h header contributed by Broadcom
sys/dev/bhnd/bhndb/bhndb_pcireg.h: * Portions of this file were derived from the bcmdevs.h header contributed by
sys/dev/bhnd/bcma/bcma_eromreg.h: * Portions of this file were derived from the aidmp.h header
sys/dev/bhnd/bcma/bcma_dmp.h: * Portions of this file were derived from the aidmp.h header
sys/dev/bhnd/bhnd_ids.h: * This file is derived from the bcmdevs.h header contributed by Broadcom
sys/dev/bhnd/bhndreg.h: * Portions of this file were derived from the sbchipc.h header contributed by
sys/dev/bhnd/siba/sibareg.h: * This file was derived from the sbconfig.h header distributed with
sys/mips/broadcom/bcm_bmips_exts.h: * This file is derived from the sbmips32.h header distributed
sys/mips/include/elf.h:/* FreeBSD specific bits - derived from FreeBSD specific files and changes to old elf.h */
sys/netipsec/keysock.c:/* This code has derived from sys/net/rtsock.c on FreeBSD2.2.5 */

So ~30 files that use some variation of this language. There's a few old drivers that we removed that used 'derived from' or 'derivative work' both of which are standard terms in this area. "subject to" is something I've not seen in the last 20 years of consulting on licenses and such. So while I'm not a lawyer, I think that experience is at least suggestive that we should avoid it. Neither of us being lawyers: you are right. However, your license didn't specify a wording, so using a similar wording to what we use elsewhere would make it easier for people. For these two reasons, I'd strongly suggest we use 'derived from' since in my opinion it matches the prior uses of this wording for what happened here.

sys/crypto/armv8/armv8_crypto_wrap.c
30

The originally the copyright was not preserved, which started this whole problem. If it had been retained like it was required, we wouldn't be here today.

Note that it is restored to this file in the lines below. There was no specification as to where and how it must be preserved, and I maintain that these lines below restore that in a way that's compliant with the license.

In D29268#656037, @jmg wrote:

the phrase "This file derived from" has no history in our tree, I'd prefer the phrase I proposed.

I'd prefer for the full path (or at least relative to sys) be included as not everyone knows our source tree well enough.

There seems to be plenty of examples for both cases. File searching is ubiquitous.

The current version looks good to me.

This revision is now accepted and ready to land.Mar 17 2021, 6:43 PM
gnn added a subscriber: gnn.

LGTM