Re: kern/46494 (cgd on 4K sector disk)

["Roland C. Dowdeswell" <elric%imrryr.org@localhost>]

On Thu, May 31, 2012 at 01:08:44PM +0200, Jan Danielsson wrote:
>

>    If dd on the cgd device appears to be working fine, the drive is
> perfectly usable if I use it without a cgd device -- what should I be
> looking at?
> 
>    For completeness, "gpt create cgd3" returns:
> 
>    gpt: unable to open device 'rcgd3d': Invalid argument
> 
>    Pointers are welcome.

I've checked in a bit of code recently that should make the gpts
work on cgd.  Please upgrade to a quite recent kernel to get that
working.

> Index: cgdvar.h
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/cgdvar.h,v
> retrieving revision 1.14
> diff -u -r1.14 cgdvar.h
> --- cgdvar.h  12 Jan 2010 21:08:09 -0000      1.14
> +++ cgdvar.h  31 May 2012 09:27:36 -0000
> @@ -81,6 +81,7 @@
>       struct cryptdata         sc_cdata;      /* crypto data */
>       struct cryptfuncs       *sc_cfuncs;     /* encryption functions */
>       struct simplelock        sc_slock;      /* our lock */
> +     unsigned                 sc_secsize;    /* sector size */
>  };
>  #endif

We should use sector size from struct dk_softc, not duplicate it here.

There's a reference to DEV_BSIZE in dksubr.c that likely needs to be
fixed as well.

> @@ -406,7 +406,7 @@
>                       return -1;
>               }
>               cgd_cipher(cs, newaddr, addr, bp->b_bcount, bn,
> -                 DEV_BSIZE, CGD_CIPHER_ENCRYPT);
> +                 cs->sc_secsize, CGD_CIPHER_ENCRYPT);
>       }

This one is interesting.  We probably do not want to use the physical
disk sector size when we're enrypting the data in the general case
as this would mean that if you change the sector size of a disk
image, dd a cgd between disks with different sector sizes, etc. it
would fail to decrypt properly (using some cipher modes, currently,
well, all of them.)

To generically solve this problem, we should likely do something like

#define CGD_BSIZE       512

at the top and use that.  We could make it configurable but this
is actually problematic as if CGD is encrypting data in 4K chunks
and writing to a disk with 512 byte sectors, you lose write atomicity
which would lead to consistency issues on crashes.  So, at the very
least, if we make it configurable then we need to enforce the
constraint that the sector size of the underlying disk >= the
encryption sector size.

--
    Roland Dowdeswell                      http://Imrryr.ORG/~elric/