[PATCH 4.4 0/3] Backports for stable 4.4 (2nd series)

[PATCH 4.4 0/3] Backports for stable 4.4 (2nd series)

[Juerg Haefliger]

Patch 1/3 is required for kexec with signed kernels.
Patch 2/3 prevents bogus reads of NVRAM which triggers false warnings.
Patch 3/3 fixes an issue with libvirt resetting the VF admin MAC to zero.

Jack Morgenstein (1):
  net/mlx4_core: Allow resetting VF admin mac to zero

Peter Jones (1):
  PKCS#7: Don't require SpcSpOpusInfo in Authenticode pkcs7 signatures

Yuval Mintz (1):
  bnx2x: Prevent false warning for lack of FC NPIV

 crypto/asymmetric_keys/pkcs7_parser.c            | 4 +---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++++
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c   | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.9.3

[PATCH 4.4 1/3] PKCS#7: Don't require SpcSpOpusInfo in Authenticode pkcs7 signatures

[Juerg Haefliger]

From: Peter Jones <pjones@redhat.com>

commit 7ee7014d0eb6bcac679c0bd5fe9ce65bc4325648 upstream

Dave Young reported:
> Hi,
>
> I saw the warning "Missing required AuthAttr" when testing kexec,
> known issue?  Idea about how to fix it?
>
> The kernel is latest linus tree plus sevral patches from Toshi to
> cleanup io resource structure.
>
> in function pkcs7_sig_note_set_of_authattrs():
>         if (!test_bit(sinfo_has_content_type, &sinfo->aa_set) ||
>             !test_bit(sinfo_has_message_digest, &sinfo->aa_set) ||
>             (ctx->msg->data_type == OID_msIndirectData &&
>              !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))) {
>                 pr_warn("Missing required AuthAttr\n");
>                 return -EBADMSG;
>         }
>
> The third condition below is true:
> (ctx->msg->data_type == OID_msIndirectData &&
>              !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))
>
> I signed the kernel with redhat test key like below:
> pesign -c 'Red Hat Test Certificate' -i arch/x86/boot/bzImage -o /boot/vmlinuz-4.4.0-rc8+ -s --force

And right he is!  The Authenticode specification is a paragon amongst
technical documents, and has this pearl of wisdom to offer:

---------------------------------
Authenticode-Specific SignerInfo UnauthenticatedAttributes Structures

  The following Authenticode-specific data structures are present in
  SignerInfo authenticated attributes.

  SpcSpOpusInfo
  SpcSpOpusInfo is identified by SPC_SP_OPUS_INFO_OBJID
  (1.3.6.1.4.1.311.2.1.12) and is defined as follows:
  SpcSpOpusInfo ::= SEQUENCE {
    programName  [0] EXPLICIT SpcString OPTIONAL,
    moreInfo     [1] EXPLICIT SpcLink OPTIONAL,
  } --#public--

  SpcSpOpusInfo has two fields:
    programName
      This field contains the program description:
      If publisher chooses not to specify a description, the SpcString
      structure contains a zero-length program name.
      If the publisher chooses to specify a
      description, the SpcString structure contains a Unicode string.
    moreInfo
      This field is set to an SPCLink structure that contains a URL for
      a Web site with more information about the signer. The URL is an
      ASCII string.
---------------------------------

Which is to say that this is an optional *unauthenticated* field which
may be present in the Authenticated Attribute list.  This is not how
pkcs7 is supposed to work, so when David implemented this, he didn't
appreciate the subtlety the original spec author was working with, and
missed the part of the sublime prose that says this Authenticated
Attribute is an Unauthenticated Attribute.  As a result, the code in
question simply takes as given that the Authenticated Attributes should
be authenticated.

But this one should not, individually.  Because it says it's not
authenticated.

It still has to hash right so the TBS digest is correct.  So it is both
authenticated and unauthenticated, all at once.  Truly, a wonder of
technical accomplishment.

Additionally, pesign's implementation has always attempted to be
compatible with the signatures emitted from contemporary versions of
Microsoft's signtool.exe.  During the initial implementation, Microsoft
signatures always produced the same values for SpcSpOpusInfo -
{U"Microsoft Windows", "http://www.microsoft.com"} - without regard to
who the signer was.

Sometime between Windows 8 and Windows 8.1 they stopped including the
field in their signatures altogether, and as such pesign stopped
producing them in commits c0c4da6 and d79cb0c, sometime around June of
2012.  The theory here is that anything that breaks with
pesign signatures would also be breaking with signtool.exe sigs as well,
and that'll be a more noticed problem for firmwares parsing it, so it'll
get fixed.  The fact that we've done exactly this bug in Linux code is
first class, grade A irony.

So anyway, we should not be checking this field for presence or any
particular value: if the field exists, it should be at the right place,
but aside from that, as long as the hash matches the field is good.

Signed-off-by: Peter Jones <pjones@redhat.com>
Tested-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Juerg Haefliger <juerg.haefliger@hpe.com>
---
 crypto/asymmetric_keys/pkcs7_parser.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 758acabf2d81..8f3056cd0399 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -547,9 +547,7 @@ int pkcs7_sig_note_set_of_authattrs(void *context, size_t hdrlen,
 	struct pkcs7_signed_info *sinfo = ctx->sinfo;
 
 	if (!test_bit(sinfo_has_content_type, &sinfo->aa_set) ||
-	    !test_bit(sinfo_has_message_digest, &sinfo->aa_set) ||
-	    (ctx->msg->data_type == OID_msIndirectData &&
-	     !test_bit(sinfo_has_ms_opus_info, &sinfo->aa_set))) {
+	    !test_bit(sinfo_has_message_digest, &sinfo->aa_set)) {
 		pr_warn("Missing required AuthAttr\n");
 		return -EBADMSG;
 	}
-- 
2.9.3

[PATCH 4.4 2/3] bnx2x: Prevent false warning for lack of FC NPIV

[Juerg Haefliger]

From: Yuval Mintz <Yuval.Mintz@qlogic.com>

commit 1e6bb1a3540fec3ef112b9a89dda88e684c3ff59 upstream.

Not all adapters have FC-NPIV configured. If bnx2fc is used with such an
adapter, driver would read irrelevant data from the the nvram and log
"FC-NPIV table with bad length..." In system logs.

Simply accept that reading '0' as the feature offset in nvram indicates
the feature isn't there and return.

Reported-by: Andrew Patterson <andrew.patterson@hpe.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Juerg Haefliger <juerg.haefliger@hpe.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2e611dc5f162..1c8123816745 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -14819,6 +14819,10 @@ static int bnx2x_get_fc_npiv(struct net_device *dev,
 	}
 
 	offset = SHMEM2_RD(bp, fc_npiv_nvram_tbl_addr[BP_PORT(bp)]);
+	if (!offset) {
+		DP(BNX2X_MSG_MCP, "No FC-NPIV in NVRAM\n");
+		goto out;
+	}
 	DP(BNX2X_MSG_MCP, "Offset of FC-NPIV in NVRAM: %08x\n", offset);
 
 	/* Read the table contents from nvram */
-- 
2.9.3

[PATCH 4.4 3/3] net/mlx4_core: Allow resetting VF admin mac to zero

[Juerg Haefliger]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

commit 6e5224224faa50ec4c8949dcefadf895e565f0d1 upstream.

The VF administrative mac addresses (stored in the PF driver) are
initialized to zero when the PF driver starts up.

These addresses may be modified in the PF driver through ndo calls
initiated by iproute2 or libvirt.

While we allow the PF/host to change the VF admin mac address from zero
to a valid unicast mac, we do not allow restoring the VF admin mac to
zero. We currently only allow changing this mac to a different unicast mac.

This leads to problems when libvirt scripts are used to deal with
VF mac addresses, and libvirt attempts to revoke the mac so this
host will not use it anymore.

Fix this by allowing resetting a VF administrative MAC back to zero.

Fixes: 8f7ba3ca12f6 ('net/mlx4: Add set VF mac address support')
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Reported-by: Moshe Levi <moshele@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Juerg Haefliger <juerg.haefliger@hpe.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 67e9633ea9c7..232191417b93 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2282,7 +2282,7 @@ static int mlx4_en_set_vf_mac(struct net_device *dev, int queue, u8 *mac)
 	struct mlx4_en_dev *mdev = en_priv->mdev;
 	u64 mac_u64 = mlx4_mac_to_u64(mac);
 
-	if (!is_valid_ether_addr(mac))
+	if (is_multicast_ether_addr(mac))
 		return -EINVAL;
 
 	return mlx4_set_vf_mac(mdev->dev, en_priv->port, queue, mac_u64);
-- 
2.9.3

Re: [PATCH 4.4 0/3] Backports for stable 4.4 (2nd series)

[Greg KH]

On Tue, Oct 18, 2016 at 11:58:51AM +0200, Juerg Haefliger wrote:
> Patch 1/3 is required for kexec with signed kernels.
> Patch 2/3 prevents bogus reads of NVRAM which triggers false warnings.
> Patch 3/3 fixes an issue with libvirt resetting the VF admin MAC to zero.

Now queued up, thanks.

greg k-h