| Message ID | 1310644590-14438-1-git-send-email-mbroz@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Alasdair Kergon |
| Headers | show |
On 07/14/2011 05:56 AM, Milan Broz wrote: > Add optional parameter field to dmcrypt table and support > "allow_discards" option. > > Discard requests bypass crypt queue processing, bio > request is simple remapped to underlying device. > > Note that discard will be never enabled by default because > of security consequences, it is up to administrator > to enable it for encrypted devices. > > (Note that userspace cryptsetup will not understand new > optional parameters yet, support for this will come later.) > > Signed-off-by: Milan Broz <mbroz@redhat.com> > [snip] Thanks very much for this patch Milan. I will try compiling it shortly and see where things go -- are you expecting this to be submitted for inclusion in mainline soon? Thanks again. From Jeff -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jul 14, 2011 at 06:56:03PM -0600, Jeff Cook wrote: > Thanks very much for this patch Milan. I will try compiling it shortly > and see where things go -- are you expecting this to be submitted for > inclusion in mainline soon? I already submitted it for linux-next, so unless people report problems which we can't fix quickly, it'll be submitted during the next merge window. Alasdair -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jul 14 2011 at 8:56pm -0400, Jeff Cook <jeff@deserettechnology.com> wrote: > On 07/14/2011 05:56 AM, Milan Broz wrote: > > Add optional parameter field to dmcrypt table and support > > "allow_discards" option. > > > > Discard requests bypass crypt queue processing, bio > > request is simple remapped to underlying device. > > > > Note that discard will be never enabled by default because > > of security consequences, it is up to administrator > > to enable it for encrypted devices. > > > > (Note that userspace cryptsetup will not understand new > > optional parameters yet, support for this will come later.) > > > > Signed-off-by: Milan Broz <mbroz@redhat.com> > > [snip] > > Thanks very much for this patch Milan. I will try compiling it shortly > and see where things go -- are you expecting this to be submitted for > inclusion in mainline soon? It is already staged for linux-next (and upstream inclusion when the 3.1 merge window opens): ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/dm-crypt-optionally-support-discard-requests.patch NOTE: this version of the patch depends on additional changes that were also staged, e.g.: ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/dm-table-share-target-argument-parsing-functions.patch -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
To check the status of a dm patch, look at patchwork in the first instance. The vast majority of dm patches are submitted to dm-devel. They are caught by patchwork here: https://patchwork.kernel.org/project/dm-devel/list/?state=* Initially status is 'New'. Patches that are assigned to 'agk' are deemed ready for me to look at. I set the status to 'Under Review' when I take a patch into my 'editing' quilt tree, which is synced onto two independent servers here: http://www.kernel.org/pub/linux/kernel/people/agk/patches/2.6/editing/ ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/ At this point the patch will be below the 'next' line (as in 'linux-next') in the quilt series file, also uploaded there. The number at the end of the line is the patchwork patch number. (Multiple numbers mean I folded patches together.) When I've reviewed a patch, I add my sign off, move it into the linux-next section and set the patchwork state to 'Awaiting Upstream'. When a group of patches enters the upstream kernel, I set the state to 'Accepted' and 'archived'. (With some minor patches that I fold into other patches, I sometimes set the state to 'Accepted' and 'archived' immediately as it's no longer worth tracking them separately. The actual patch included upstream is often not identical to the one in patchwork as I can make small changes to it for many reasons during review.) Periodically I take a backup of the patch directory (tagged with the kernel version) and remove some patches no longer needed (including patches now upstream). (Go up the directory hierarchy to see these backups.) Alasdair -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 07/14/2011 07:13 PM, Mike Snitzer wrote: > On Thu, Jul 14 2011 at 8:56pm -0400, > Jeff Cook <jeff@deserettechnology.com> wrote: >> Thanks very much for this patch Milan. I will try compiling it shortly >> and see where things go -- are you expecting this to be submitted for >> inclusion in mainline soon? > > It is already staged for linux-next (and upstream inclusion when the 3.1 > merge window opens): > ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/dm-crypt-optionally-support-discard-requests.patch > > NOTE: this version of the patch depends on additional changes that were > also staged, e.g.: > ftp://sources.redhat.com/pub/dm/patches/2.6-unstable/editing/patches/dm-table-share-target-argument-parsing-functions.patch Sorry to be a pest, but I have a few more basic questions about the application of these patches and their use on my encrypted disk. First of all, I attempted to apply these patches to 3.0-rc6 and HEAD (slightly after 3.0-rc7) and they did not apply cleanly. I was able to get a correct application once, but then the compile failed, assumedly due to a mispatching. Now that 3.0 has been tagged, should I assume that the patches will apply cleanly to that tree? I tried both manual patch via /usr/bin/patch and git apply. I suppose I can just wait for 3.1-rc1 if I must. Secondly, I am not sure how I go about modifying the crypt table mapping to activate the discard pass-through option. I recognize that Milan's commit message indicates support is missing in cryptsetup -- does this mean that a patch must be written to cryptsetup before the new option can be activated or can this be accessed by the lower-level dmsetup tool? I would guess the latter, though I don't imagine it would be difficult to patch cryptsetup if needed (I haven't looked at the code yet, though). Is the mapping table modifiable at mount time, so this feature can be activated on existing dm-crypt partitions, or would this require the partition to be formatted with different options? If it can be modified on mount, how do we set those parameters? Can I just boot off of a live medium, mount manually with (patched) cryptsetup luksOpen, and have the change kept on disk, or do I need to do something else? Apologies for these noob-ish questions -- just thought I'd get much better information straight from the source. All help is appreciated, and I would like to express my gratitude once more for the construction of this patch in the first place. Thanks Milan, Mike, and anyone else involved for your effort, this will be a big help to a lot of people I think. I almost didn't buy a SSD due to the lack of TRIM support in dm-crypt. :) Thanks again. From Jeff -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 07/22/2011 11:41 AM, Jeff Cook wrote: > First of all, I attempted to apply these patches to 3.0-rc6 and HEAD > (slightly after 3.0-rc7) and they did not apply cleanly. I was able to > get a correct application once, but then the compile failed, assumedly > due to a mispatching. Now that 3.0 has been tagged, should I assume that > the patches will apply cleanly to that tree? I tried both manual patch > via /usr/bin/patch and git apply. I suppose I can just wait for 3.1-rc1 > if I must. So better wait for rc1, patch is already in linux-next. My original patch was not using new table parsing function so I have no idea which patches need to be backported now. > Secondly, I am not sure how I go about modifying the crypt table mapping > to activate the discard pass-through option. You can compile upstream cryptsetup, I already added flag to support discard. But there is intentional library version bump because I removed old api symbols and also I want to implement some features before new version is released, so use with care - it is not stable code yet. > I recognize that Milan's commit message indicates support is missing in > cryptsetup -- does this mean that a patch must be written to cryptsetup > before the new option can be activated or can this be accessed by the > lower-level dmsetup tool? You can use dmsetup for now, I sent description how to do that to dmcrypt list. http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/5166 > I almost didn't buy a SSD due to the lack of TRIM support in > dm-crypt. :) TRIM is incredibly overrated:-) Milan -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/Documentation/device-mapper/dm-crypt.txt b/Documentation/device-mapper/dm-crypt.txt index 6b5c42d..08a36a7 100644 --- a/Documentation/device-mapper/dm-crypt.txt +++ b/Documentation/device-mapper/dm-crypt.txt @@ -4,7 +4,7 @@ dm-crypt Device-Mapper's "crypt" target provides transparent encryption of block devices using the kernel crypto API. -Parameters: <cipher> <key> <iv_offset> <device path> <offset> +Parameters: <cipher> <key> <iv_offset> <device path> <offset> [<#opt_params> <opt_params>] <cipher> Encryption cipher and an optional IV generation mode. @@ -37,6 +37,23 @@ Parameters: <cipher> <key> <iv_offset> <device path> <offset> <offset> Starting sector within the device where the encrypted data begins. +<#opt_params> + Number of optional parameters. If there are no optional parameters, + the optional paramaters section can be skipped or #opt_params can be zero. + Otherwise #opt_params indicates count of following arguments. + + Examples of optional parameters section: + 1 allow_discards + +allow_discards + The block discards requests are passed through the crypt device. + (Default is to block discards requests.) + + WARNING: allowing discard on encrypted device has serious irreversible + security consequences, discarded blocks can be easily located on device + later. This can lead to leak of information from ciphertext device + (unique pattern for detecting filesystem type, used space etc). + Example scripts =============== LUKS (Linux Unified Key Setup) is now the preferred way to set up disk diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index c8827ff..2747483 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1575,11 +1575,11 @@ bad_mem: static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv) { struct crypt_config *cc; - unsigned int key_size; + unsigned int key_size, opt_params; unsigned long long tmpll; int ret; - if (argc != 5) { + if (argc < 5) { ti->error = "Not enough arguments"; return -EINVAL; } @@ -1648,6 +1648,26 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv) } cc->start = tmpll; + /* Optional parameters */ + if (argc > 5) { + if (sscanf(argv[5], "%u", &opt_params) < 0) { + ti->error = "Invalid optional parameters"; + goto bad; + } + + if (opt_params + 6 > argc) { + ti->error = "Arguments do not agree with counts given"; + goto bad; + } + + if (opt_params == 1 && !strcmp(argv[6], "allow_discards")) + ti->num_discard_requests = 1; + else if (opt_params) { + ti->error = "Invalid optional parameters"; + goto bad; + } + } + ret = -ENOMEM; cc->io_queue = alloc_workqueue("kcryptd_io", WQ_NON_REENTRANT| @@ -1682,9 +1702,16 @@ static int crypt_map(struct dm_target *ti, struct bio *bio, struct dm_crypt_io *io; struct crypt_config *cc; - if (bio->bi_rw & REQ_FLUSH) { + /* + * If bio is REQ_FLUSH or REQ_DISCARD, just bypass crypt queues. + * - for REQ_FLUSH device-mapper core ensures that no IO is in-flight + * - for REQ_DISCARD caller must use flush if IO ordering matters + */ + if (unlikely(bio->bi_rw & (REQ_FLUSH | REQ_DISCARD))) { cc = ti->private; bio->bi_bdev = cc->dev->bdev; + if (bio_sectors(bio)) + bio->bi_sector = cc->start + dm_target_offset(ti, bio->bi_sector); return DM_MAPIO_REMAPPED; } @@ -1727,6 +1754,10 @@ static int crypt_status(struct dm_target *ti, status_type_t type, DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset, cc->dev->name, (unsigned long long)cc->start); + + if (ti->num_discard_requests) + DMEMIT(" 1 allow_discards"); + break; } return 0; @@ -1823,7 +1854,7 @@ static int crypt_iterate_devices(struct dm_target *ti, static struct target_type crypt_target = { .name = "crypt", - .version = {1, 10, 0}, + .version = {1, 11, 0}, .module = THIS_MODULE, .ctr = crypt_ctr, .dtr = crypt_dtr,
Add optional parameter field to dmcrypt table and support "allow_discards" option. Discard requests bypass crypt queue processing, bio request is simple remapped to underlying device. Note that discard will be never enabled by default because of security consequences, it is up to administrator to enable it for encrypted devices. (Note that userspace cryptsetup will not understand new optional parameters yet, support for this will come later.) Signed-off-by: Milan Broz <mbroz@redhat.com> --- Documentation/device-mapper/dm-crypt.txt | 19 +++++++++++++- drivers/md/dm-crypt.c | 39 ++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-)