Re: [PATCH v7 06/13] x86: Add early SHA support for Secure Launch early measurements

2023-11-11 Thread James Bottomley 
On Sat, 2023-11-11 at 18:19 +, Andrew Cooper wrote:
> On 11/11/2023 5:44 pm, Eric Biggers wrote:
> > On Fri, Nov 10, 2023 at 05:27:44PM -0500, Ross Philipson wrote:
> > >  arch/x86/boot/compressed/early_sha1.c   | 12 
> > >  lib/crypto/sha1.c   | 81
> > > +
> > It's surprising to still see this new use of SHA-1 after so many
> > people objected to it in the v6 patchset.  It's also frustrating
> > that the SHA-1 support is still being obfuscated by being combined
> > in one patch with SHA-2 support, perhaps in an attempt to conflate
> > the two algorithms and avoid having to give a rationale for the
> > inclusion of SHA-1.  Finally, new functions should not be added to
> > lib/crypto/sha1.c unless those functions have multiple users.
> 
> The rational was given.  Let me reiterate it.
> 
> There are real TPMs in the world that can't use SHA-2.  The use of
> SHA-1 is necessary to support DRTM on such systems, and there are
> real users of such configurations.

Given that TPM 2.0 has been shipping in bulk since Windows 10 (2015)
and is required for Windows 11 (2021), are there really such huge
numbers of TPM 1.2 systems involved in security functions?

> DRTM with SHA-1-only is a damnsight better than no DTRM, even if SHA-
> 1 is getting a little long in the tooth.

That's not the problem.  The problem is that sha1 is seen as a
compromised algorithm by NIST which began deprecating it in 2011 and is
now requiring it to be removed from all systems supplied to the US
government by 2030

https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm

That means we have to control all uses of sha1 in the kernel and have
an option to build without it.  FIPS has an even tighter timetable: it
requires sha1 to be out by 2025.

> So unless you have a credible plan to upgrade every non-SHA-2 TPM in
> the world, you are deliberately breaking part of the usecase paying
> for the effort of trying to upstream DRTM support into Linux.

Given that most CSOs follow NIST and FIPS it seems a little strange
that there would be a huge demand for such an intricate security
protocol as Dynamic Launch on a system that can't be FIPS 140-3
certified.

James

Re: [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-15 Thread James Bottomley 
On Tue, 2020-09-15 at 08:27 +0200, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 08:20:18AM -0700, James Bottomley wrote:
> > If you're going to change the macros from taking a device to taking
> > a hostdata structure then the descriptive argument name needs to
> > change ... it can't be dev anymore.  I'm happy with it simply
> > becoming 'h' if hostdata is too long.
> > 
> > I already asked for this on the first go around:
> 
> And I did rename them, those hunks just accidentally slipped into
> patch 12 instead of this one.  Fixed for the next versions.

Ah, yes, found it ... thanks for doing that!

James

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 07/17] 53c700: improve non-coherent DMA handling

2020-09-14 Thread James Bottomley 
On Mon, 2020-09-14 at 16:44 +0200, Christoph Hellwig wrote:
> @@ -429,7 +430,7 @@ struct NCR_700_Host_Parameters {
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   __u32 val =
> bS_to_cpu((script)[A_##symbol##_used[i]]) + da; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], )); \
>   } \
> @@ -441,7 +442,7 @@ struct NCR_700_Host_Parameters {
>   dma_addr_t da = value; \
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   (script)[A_##symbol##_used[i]] = bS_to_host(da); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], )); \
>   } \
> @@ -456,7 +457,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0xff00; \
>   val |= ((value) & 0xff) << 16; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching ID field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \
> @@ -470,7 +471,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0x; \
>   val |= ((value) & 0x); \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching short field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \

If you're going to change the macros from taking a device to taking a
hostdata structure then the descriptive argument name needs to change
... it can't be dev anymore.  I'm happy with it simply becoming 'h' if
hostdata is too long.

I already asked for this on the first go around:

https://lore.kernel.org/alsa-devel/1598971960.4238.5.ca...@hansenpartnership.com/

James

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 07/28] 53c700: improve non-coherent DMA handling

2020-09-01 Thread James Bottomley 
On Tue, 2020-09-01 at 16:05 +0100, Matthew Wilcox wrote:
> On Tue, Sep 01, 2020 at 07:52:40AM -0700, James Bottomley wrote:
> > I think this looks mostly OK, except for one misnamed parameter
> > below. Unfortunately, the last non-coherent parisc was the 700
> > series and I no longer own a box, so I can't test that part of it
> > (I can fire up the C360 to test it on a coherent arch).
> 
> I have a 715/50 that probably hasn't been powered on in 15 years if
> you need something that old to test on (I believe the 725/100 uses
> the 7100LC and so is coherent).  I'll need to set up a cross-compiler 
> ...

I'm not going to say no to actual testing, but it's going to be a world
of pain getting something so old going.  I do have a box of older
systems I keep for architectural testing that I need to rummage around
in ... I just have a vague memory that my 715 actually caught fire a
decade ago and had to be disposed of.

James

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 07/28] 53c700: improve non-coherent DMA handling

2020-09-01 Thread James Bottomley 
On Wed, 2020-08-19 at 08:55 +0200, Christoph Hellwig wrote:
> Switch the 53c700 driver to only use non-coherent descriptor memory
> if it really has to because dma_alloc_coherent fails.  This doesn't
> matter for any of the platforms it runs on currently, but that will
> change soon.
> 
> To help with this two new helpers to transfer ownership to and from
> the device are added that abstract the syncing of the non-coherent
> memory. The two current bidirectional cases are mapped to transfers
> to the device, as that appears to what they are used for.  Note that
> for parisc, which is the only architecture this driver needs to use
> non-coherent memory on, the direction argument of dma_cache_sync is
> ignored, so this will not change behavior in any way.

I think this looks mostly OK, except for one misnamed parameter below. 
Unfortunately, the last non-coherent parisc was the 700 series and I no
longer own a box, so I can't test that part of it (I can fire up the
C360 to test it on a coherent arch).

[...]
> diff --git a/drivers/scsi/53c700.h b/drivers/scsi/53c700.h
> index 05fe439b66afe5..0f545b05fe611d 100644
> --- a/drivers/scsi/53c700.h
> +++ b/drivers/scsi/53c700.h
> @@ -209,6 +209,7 @@ struct NCR_700_Host_Parameters {
>  #endif
>   __u32   chip710:1;  /* set if really a 710 not
> 700 */
>   __u32   burst_length:4; /* set to 0 to disable
> 710 bursting */
> + __u32   noncoherent:1;  /* needs to use non-
> coherent DMA */
>  
>   /* NOTHING BELOW HERE NEEDS ALTERING */
>   __u32   fast:1; /* if we can alter the
> SCSI bus clock
> @@ -429,7 +430,7 @@ struct NCR_700_Host_Parameters {
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   __u32 val =
> bS_to_cpu((script)[A_##symbol##_used[i]]) + da; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], )); \
>   } \
> @@ -441,7 +442,7 @@ struct NCR_700_Host_Parameters {
>   dma_addr_t da = value; \
>   for(i=0; i< (sizeof(A_##symbol##_used) / sizeof(__u32));
> i++) { \
>   (script)[A_##symbol##_used[i]] = bS_to_host(da); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching %s at %d to %pad\n", \
>  #symbol, A_##symbol##_used[i], )); \
>   } \
> @@ -456,7 +457,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0xff00; \
>   val |= ((value) & 0xff) << 16; \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching ID field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \
> @@ -470,7 +471,7 @@ struct NCR_700_Host_Parameters {
>   val &= 0x; \
>   val |= ((value) & 0x); \
>   (script)[A_##symbol##_used[i]] = bS_to_host(val); \
> - dma_cache_sync((dev),
> &(script)[A_##symbol##_used[i]], 4, DMA_TO_DEVICE); \
> + dma_sync_to_dev((dev),
> &(script)[A_##symbol##_used[i]], 4); \
>   DEBUG((" script, patching short field %s at %d to
> 0x%x\n", \
>  #symbol, A_##symbol##_used[i], val)); \
>   } \

These macro arguments need updating.  Since you changed the input from
hostdata->dev to hostdata, leaving the macro argument as dev is simply
misleading.  It needs to become hostdata or h.

James

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 7/8] parisc: don't set ARCH_NO_COHERENT_DMA_MMAP

2019-08-15 Thread James Bottomley 
On Thu, 2019-08-08 at 19:00 +0300, Christoph Hellwig wrote:
> parisc is the only architecture that sets ARCH_NO_COHERENT_DMA_MMAP
> when an MMU is enabled.  AFAIK this is because parisc CPUs use VIVT
> caches,

We're actually VIPT but the same principle applies.

>  which means exporting normally cachable memory to userspace is
> relatively dangrous due to cache aliasing.
> 
> But normally cachable memory is only allocated by dma_alloc_coherent
> on parisc when using the sba_iommu or ccio_iommu drivers, so just
> remove the .mmap implementation for them so that we don't have to set
> ARCH_NO_COHERENT_DMA_MMAP, which I plan to get rid of.

So I don't think this is quite right.  We have three architectural
variants essentially (hidden behind about 12 cpu types):

   1. pa70xx: These can't turn off page caching, so they were the non
  coherent problem case
   2. pa71xx: These can manufacture coherent memory simply by turning off
  the cache on a per page basis
   3. pa8xxx: these have a full cache flush coherence mechanism.

(I might have this slightly wrong: I vaguely remember the pa71xxlc
variants have some weird cache quirks for DMA as well)

So I think pa70xx we can't mmap.  pa71xx we can provided we mark the
page as uncached ... which should already have happened in the allocate
and pa8xxx which can always mmap dma memory without any special tricks.

James

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH V4 1/2] ACPI / EC: Fix broken 64bit big-endian users of 'global_lock'

2015-09-26 Thread James Bottomley 
On Fri, 2015-09-25 at 22:58 +0200, Rafael J. Wysocki wrote:
> On Friday, September 25, 2015 01:25:49 PM Viresh Kumar wrote:
> > On 25 September 2015 at 13:33, Rafael J. Wysocki  wrote:
> > > You're going to change that into bool in the next patch, right?
> > 
> > Yeah.
> > 
> > > So what if bool is a byte and the field is not word-aligned
> > 
> > Its between two 'unsigned long' variables today, and the struct isn't 
> > packed.
> > So, it will be aligned, isn't it?
> > 
> > > and changing
> > > that byte requires a read-modify-write.  How do we ensure that things 
> > > remain
> > > consistent in that case?
> > 
> > I didn't understood why a read-modify-write is special here? That's
> > what will happen
> > to most of the non-word-sized fields anyway?
> > 
> > Probably I didn't understood what you meant..
> 
> Say you have three adjacent fields in a structure, x, y, z, each one byte 
> long.
> Initially, all of them are equal to 0.
> 
> CPU A writes 1 to x and CPU B writes 2 to y at the same time.
> 
> What's the result?

I think every CPU's  cache architecure guarantees adjacent store
integrity, even in the face of SMP, so it's x==1 and y==2.  If you're
thinking of old alpha SMP system where the lowest store width is 32 bits
and thus you have to do RMW to update a byte, this was usually fixed by
padding (assuming the structure is not packed).  However, it was such a
problem that even the later alpha chips had byte extensions.

James


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] hpsa: fix uninitialized trans_support in hpsa_put_ctlr_into_performant_mode()

2014-04-14 Thread James Bottomley 
Your subject line is very tame.  It should be the one line summary of
why we apply the patch, so it should read something like

 hpsa: fix NULL deref in performant mode

On Thu, 2014-04-10 at 17:17 -0500, scame...@beardog.cce.hp.com wrote:
 Without this, you'll see a null pointer dereference in
 hpsa_enter_performant_mode().

The description should be more comprehensible.

I'm clear that the use before initialisation is a bug ... I'm less clear
on why it causes an oops.

 Signed-off-by: Stephen M. Cameron scame...@beardog.cce.hp.com
 ---
  drivers/scsi/hpsa.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
 index 8cf4a0c..ef4dfdd 100644
 --- a/drivers/scsi/hpsa.c
 +++ b/drivers/scsi/hpsa.c
 @@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
 ctlr_info *h)
   if (hpsa_simple_mode)
   return;
  
 + trans_support = readl((h-cfgtable-TransportSupport));
 + if (!(trans_support  PERFORMANT_MODE))
 + return;
 +
   /* Check for I/O accelerator mode support */
   if (trans_support  CFGTBL_Trans_io_accel1) {
   transMethod |= CFGTBL_Trans_io_accel1 |

Shouldn't you be moving this check from its previous location, rather
than adding a new one that makes the original obsolete?

James

---

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8cf4a0c..9a6e4a2 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -7463,6 +7463,10 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
ctlr_info *h)
if (hpsa_simple_mode)
return;
 
+   trans_support = readl((h-cfgtable-TransportSupport));
+   if (!(trans_support  PERFORMANT_MODE))
+   return;
+
/* Check for I/O accelerator mode support */
if (trans_support  CFGTBL_Trans_io_accel1) {
transMethod |= CFGTBL_Trans_io_accel1 |
@@ -7479,10 +7483,6 @@ static void hpsa_put_ctlr_into_performant_mode(struct 
ctlr_info *h)
}
 
/* TODO, check that this next line h-nreply_queues is correct */
-   trans_support = readl((h-cfgtable-TransportSupport));
-   if (!(trans_support  PERFORMANT_MODE))
-   return;
-
h-nreply_queues = h-msix_vector  0 ? h-msix_vector : 1;
hpsa_get_max_perf_mode_cmds(h);
/* Performant mode ring buffer and supporting data structures */


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: How to specify IOMMU'able devices in DT (was: [RFC 0/5] ARM: dma-mapping: New dma_map_ops to control IOVA more precisely)

2012-09-24 Thread James Bottomley 
On Mon, 2012-09-24 at 12:04 +0300, Hiroshi Doyu wrote:
 diff --git a/drivers/base/platform.c b/drivers/base/platform.c
 index a1a7225..9eae3be 100644
 --- a/drivers/base/platform.c
 +++ b/drivers/base/platform.c
 @@ -21,6 +21,8 @@
  #include linux/slab.h
  #include linux/pm_runtime.h
 
 +#include asm/dma-iommu.h
 +
  #include base.h
 
  #define to_platform_driver(drv)(container_of((drv), struct
 platform_driver, \
 @@ -305,8 +307,19 @@ int platform_device_add(struct platform_device
 *pdev)
  dev_name(pdev-dev), dev_name(pdev-dev.parent));
 
 ret = device_add(pdev-dev);
 -   if (ret == 0)
 -   return ret;
 +   if (ret)
 +   goto failed;
 +
 +#ifdef CONFIG_PLATFORM_ENABLE_IOMMU
 +   if (platform_bus_type.map  !pdev-dev.archdata.mapping) {
 +   ret = arm_iommu_attach_device(pdev-dev,
 + platform_bus_type.map);
 +   if (ret)
 +   goto failed;

This is horrible ... you're adding an architecture specific callback
into our generic code; that's really a no-no.  If the concept of
CONFIG_PLATFORM_ENABE_IOMMU is useful to more than just arm, then this
could become a generic callback.

James


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu