Re: [dm-devel] [RFC PATCH v10 11/17] dm-verity: consume root hash digest and signature data via LSM hook

[Alasdair G Kergon]

On Tue, Aug 08, 2023 at 03:45:03PM -0700, Fan Wu wrote:
> On Tue, Jul 25, 2023 at 04:43:48PM -0400, Paul Moore wrote:
> > Where would the finalize() hook be called?
> 
> It is in the __bind function in drivers/md/dm.c, calling just before 
> rcu_assign_pointer(md->map, (void *)t) which activates the inactive table.
 
That would be after the existing commit point, meaning the table swap
cannot be cancelled there, so is the finalize() you are proposing void()
i.e. designed so it always succeeds?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm: remove unneeded variable

[Alasdair G Kergon]

On Wed, Jun 14, 2023 at 11:12:19AM +0800, baomingtong...@208suo.com wrote:
>在 2023-06-14 10:42，Alasdair G Kergon 写道：
> >  Did the patched code compile OK for you?

>Yes!
 
So what is the definition of DMEMIT that your compiler uses and why?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm: remove unneeded variable

[Alasdair G Kergon]

On Wed, Jun 14, 2023 at 10:10:33AM +0800, baomingtong...@208suo.com wrote:
> fix the following coccicheck warning:
> drivers/md/dm-snap-persistent.c:909:14-16: Unneeded variable: "sz".
 
> - unsigned int sz = 0;
 
> - return sz;
> + return 0;
 
Did the patched code compile OK for you?

The semantics of the DMEMIT macro are perhaps a tad unexpected.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] A kernel panic (or soft lockup) due to stack overflow by recursive dm-table reload

[Alasdair G Kergon]

On Thu, Aug 25, 2022 at 12:49:06AM +0800, Coly Li wrote:
> 5, reload dm table for dm-0
> # cat dm-table-reload | dmsetup reload /dev/dm-0
> And the content of dm-table-reload is,
>   0 1 linear /dev/dm-0 0
>   1 181065566 linear /dev/dm-0 1 
 
> 1) Does anyone observe or encounter such panic or deadlock before?
> 2) To permit recursive dm-table reload, is it a bug or just as-designed ?
 
It's one of those 'That is a stupid thing to do!' situations.
Don't do it!

We have some detection for recursion in our userspace code but we can't
catch everything.

But arguably that specific case of a self-reference is an easy one to
detect kernel-side - table_load could ensure dm_table_get_devices() does
not include the device itself - so it might be worth patching.

Alasdair
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] dm-devel mailing list messages not sent to everyone last week

[Alasdair G Kergon]

Some curious happenings led to the configuration and membership
of the dm-devel mailing list getting wound back to its state in 2004
just over a week ago.  It was restored yesterday, but you might want to
look at the list archives to check for any messages you missed:

  https://listman.redhat.com/archives/dm-devel/2022-August/date.html

Longer version.

In 2001, mailman introduced some code to change the format in which
it stored its list information.  It took a few years for this
to reach deployed distros.  It stored the new data for each list under a
new filename.  Its code looks first for the new file and if it's not
there, uses the old one and converts it into the new format.  After it
has run, the new file will be present so it will just use that from
then on.

Crucially, if the new file is present but it fails to read its contents
successfully, it also falls back and uses the old file if it can.  (I'm
guessing this was so it could handle interruption during conversion
transparently.) 

The old file was still present for the dm-devel mailing list, and after
a reboot on 11th August, the system was unable to read the current file
(and its backup) and fell back to using the file from 2004, reconverting
it!

People started reporting some odd problems with the mailing list but it
took several days for us to realise what was going on.  We don't know
the root cause of the corruption but are continuing to investigate.

Lessons so far:
1. If you code a transparent upgrade mechanism, make sure it can only
get triggered once, so it cannot repeat itself at a random time many
years later.

2. If you get an error processing a data file, it might be better just
to stop rather than to assume continuing on with an older copy will be
just fine.

3. Make full use of atomic filesystem transactions when updating files
and don't disregard the need to flush in all the right places.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm: introduce a no open flag for deferred remove

[Alasdair G Kergon]

On Mon, Jan 24, 2022 at 10:25:47AM -0500, Brian Geffon wrote:
> Thank you for looking at this. There are a few reasons this might be
> useful, the first is if you're trying to speed up a graceful teardown
> of the device by informing userspace that this device is going to be
> removed in the near future. Another might be on systems where it might
> be worthwhile to not have users with CAP_DAC_OVERRIDE be able to open
> the device. The logic on this second case is that, suppose you have a
> dm-crypt block device which is backing swap, the data on this device
> is ephemeral so a flow might be to setup swap followed by dmsetup
> remove --deferred /dev/mapper/encrypted-swap. This will guarantee that
> as soon as swap is torn down the encrypted block device is dropped,
> additionally with this new flag you'll be guaranteed that there can be
> no further opens on it.
 
And is that the reason you propose this?
- You want a special exclusive 'one time open' device that 
  self-destructs when closed?

> No, this is fully backwards compatible with the current deferred
> remove behavior, it's not required. Additionally, since on the actual
> remove userspace would receive an -ENXIO already once the remove
> process has started it seems reasonable to return -ENXIO in the
> deferred remove case when this flag is enabled.
 
Well I feel it does break existing semantics which is why we wrote
the code the way we did.  The state can be long-lived, the code
that has it open might legitimately want to open it again in
parallel etc. - in general this seems a bad idea.

But if the reason for this is basically "make it harder for 
anything else to access my encrypted swap" and to deliberately
prevent access, then let's approach the requirement from that angle.
Are there alternative implementations with interventions at different
points?  Are there similar requirements for devices that don't need
deferred remove?  If this is indeed the best place to insert this type
of restriction, then we should label it and document it accordingly so
people don't mistakenly use it for the 'normal' case.  (We always keep
libdevmapper and dmsetup in sync with kernel interface extensions, so
we'd seek a tiny patch to do that too.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm: introduce a no open flag for deferred remove

[Alasdair G Kergon]

On Mon, Jan 24, 2022 at 07:02:09AM -0800, Brian Geffon wrote:
> When a device is being removed with deferred remove it's
> still possible to open and use the device. This change
> introduces a flag called DM_DEFERRED_REMOVE_NO_OPEN_FLAG
> which when used with DM_DEFERRED_REMOVE will cause any
> new opens to fail with -ENXIO.
 
What is the need for this?
Does it break any semantics assumed by userspace?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] announcing the dm-update target

[Alasdair G Kergon]

On Wed, Nov 24, 2021 at 12:38:31AM +, Alasdair G Kergon wrote:
> There seems to be a general industry movement towards edge, attestation,
> trusted boot, nested systems, confidential computing, containers,
>  etc. and I think this type of
> device-mapper target might play a useful role as one of the low-level
> components involved in building up support for some of those scenarios.
> (Just as we recently began adding support for attestation.)

For example, I think we've already made good progress towards
standardising the industry around dm-verity.  (At least, I keep
on encountering it being used in various different places, rather
than people attempting to develop their own version.)  We extended
that concept with dm-integrity but I'm less sure about how widely
that has been taken up so far.

For edge systems needing unattended remote reliable updates, our
high-level challenge here is: Can we together develop a decent and
efficient solution for OTA updates that the industry will be happy to
standardise around?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] announcing the dm-update target

[Alasdair G Kergon]

On Tue, Nov 23, 2021 at 04:07:21PM -0800, Akilesh Kailash wrote:
> Why not extend dm-snapshot; in that way you can have the existing
> kernel COW format + support compression.

In short, this is quite a focussed requirement and it doesn't seem worth
compromising on the goals by attempting to shoe-horn it into a framework
that was designed to fit around some quite different constraints twenty
years ago.

> I understand this for read-only but if dm-snapshot supports it,
> existing users  can have the
> compression feature if required.

I doubt that anyone wants to develop dm-snapshot any further.  It's only
really good for small short-term snapshots where performance and memory
usage don't matter.  The more-sophisticated thin provisioning approach
replaced it.

> Also, I am curious what are the other real world use case here apart
> from Android ?

There seems to be a general industry movement towards edge, attestation,
trusted boot, nested systems, confidential computing, containers,
 etc. and I think this type of
device-mapper target might play a useful role as one of the low-level
components involved in building up support for some of those scenarios.
(Just as we recently began adding support for attestation.)
 
Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] announcing the dm-update target

[Alasdair G Kergon]

On Tue, Nov 23, 2021 at 02:27:18PM -0800, Akilesh Kailash wrote:
> On Tue, Nov 23, 2021 at 1:03 PM Mikulas Patocka  wrote:
> Older devices do not get new kernel features, only LTS updates. On the
> other hand, by having
> it in user-space, we have more control by adding in new features. For
> ex: On Android S, we
> introduced the first version of Android COW format. Now in Android T,
> we plan to support
> new features by having XOR compression (which saves more space).

While you are developing this, sure, you're finding new ways that
make significant space savings and want to roll them out easily
and the userspace approach helps you to do that.

But the law of diminishing returns will eventually kick in, where you
have reached a format that provides "good enough" space savings and then
reducing the runtime performance penalty will become the overriding
concern and that'll point back to an in-kernel solution.

So that's the end point I think we are aiming for here.  Combining the
requirements to find a sweet spot between space saving and performance.
 
By then, the ability still to make a tweak that saves a tiny bit more
space isn't going to be worth paying an ongoing performance penalty
for.  (And there could still be some sort of ebpf-style option.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] multipath-tools: dm-devel is a closed ml

[Alasdair G Kergon]

On Fri, Oct 08, 2021 at 05:06:26PM +0200, Xose Vazquez Perez wrote:
> Just for subscribers

Although we manually moderate non-subscribers so their messages
do get through, often a bit delayed.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk

[Alasdair G Kergon]

On Tue, Aug 10, 2021 at 03:12:27PM +0200, Peter Rajnoha wrote:
> (I'm not counting the very corner use case of
> 'dmsetup --addnodeoncreate --verifyudev' which now ends up with a dev node
> in /dev that logically returns -ENODEV when accessed instead of zero-sized
> device as it was before.)
 
Yes.  That facility was provided to assist people having to work with
old or incorrect code or misconfigured systems and breaking it in this
way shouldn't be a concern.  (We could possibly still patch it up to
continue to do the best thing after the patchset goes in.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] use regular gendisk registration in device mapper v2

[Alasdair G Kergon]

On Tue, Aug 10, 2021 at 01:36:08AM +0100, Alasdair G Kergon wrote:
> On Wed, Aug 04, 2021 at 11:41:39AM +0200, Christoph Hellwig wrote:
> > allows device mapper to use the normal scheme
> > of calling add_disk when it is ready to accept I/O.
> For clarity, even after this patchset, the device is not ready to accept
> I/O when add_disk is called.  

The question then arises: could we go beyond this patchset and move the
add_disk further to the first resume to make the statement true?  (From
step 2 to 3 in my earlier response. DM_TABLE_CLEAR then also enters the
mix for testing.)

In the early days, in practice userspace did have to resume a device
before it could be referenced in a table and lvm2 and other tools were
designed with that in mind - they should always resume a device before
loading a table that references it.  This was because the device
reference performed a size check - to make sure the access was within
the device, and the device size isn't defined until a table becomes live
when the device is resumed.  But some multipath tables had to be set up
referencing devices with not-yet-defined sizes, so the code got relaxed
to accept references to zero-sized devices.  (At the back of my mind I
think there was some non-multipath code that found this a convenient
short-cut too.)

So since this "must resume before referencing in a table" hasn't been
enforced for so long, I can't really say how much userspace code, if
any, might now not be doing it.  We and others would need to do some
testing to see if we could get away with making such a change.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] use regular gendisk registration in device mapper v2

[Alasdair G Kergon]

On Wed, Aug 04, 2021 at 11:41:39AM +0200, Christoph Hellwig wrote:
> allows device mapper to use the normal scheme
> of calling add_disk when it is ready to accept I/O.

For clarity, even after this patchset, the device is not ready to accept
I/O when add_disk is called.  It is ready to accept I/O later if a 
'resume' happens triggering the 'change' uevent that userspace reacts
to by setting up the /dev entries for it.
 
Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk

[Alasdair G Kergon]

On Tue, Aug 10, 2021 at 12:31:43AM +0100, Alasdair G Kergon wrote:
> When loading tables, our tools also always refer to devices using
> the 'major:minor' format, which isn't affected, rather than using

Wrong - that is also affected.

So there is a new general constraint that a table must be loaded into a
device before another device's table can reference that device.  (The
stacked device handling in lvm2 as supported by libdevmapper should
always be doing this.)

(The original implementation had to be a bit loose to accommodate
multipath device paths that were essentially placeholders at the point
they got set up.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk

[Alasdair G Kergon]

On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote:
> device mapper is currently the only outlier that tries to call
> register_disk after add_disk, leading to fairly inconsistent state
> of these block layer data structures.  

> Note that this introduces a user visible change: the dm kobject is
> now only visible after the initial table has been loaded.

Indeed.  We should try to document the userspace implications of this
change in a bit more detail.  While lvm2 and any other tools that
followed our recommendations about how to use dm should be OK, there's
always the chance that some other less robustly-written code will need
to make adjustments.

Currently to make a dm device, 3 ioctls are called in sequence:

1. DM_DEV_CREATE  - triggers 'add' uevents
2. DM_TABLE_LOAD
3. DM_SUSPEND - triggers 'change' uevent

After this patch we have:

1. DM_DEV_CREATE  
2. DM_TABLE_LOAD  - triggers 'add' uevents
3. DM_SUSPEND - triggers 'change' uevent

The equivalent dmsetup commands for a simple test device are
0. udevadm monitor --kernel --env &   # View the uevents as they happen
1. dmsetup create dev1 --notable
2. dmsetup load --table "0 1 error" dev1
3. dmsetup resume dev1

  => Anyone with a udev rule that relies on 'add' needs to check if they
 need to change their code.

The udev rules that lvm2 uses to synchronise what it is doing rely
only on the 'change' event - which is not moving.  The 'add' event
gets ignored.  

When loading tables, our tools also always refer to devices using
the 'major:minor' format, which isn't affected, rather than using
pathnames in /dev which might not exist now after this change if a table
hasn't been loaded into a referenced device yet.  Previously this was
permissible but we always recommended against it to avoid a pointless
pathname lookup that's subject to races and delays.

So again, any tools that followed our recommendations ought to be
unaffected.

Here's an example of poor code that previously worked but will fail now:
  dmsetup create dev1 --notable
  dmsetup create dev2 --notable
  dmsetup ls  <-- get the minor number of dev1 (say it's 1 corresponding
to dm-1)
  dmsetup load dev2 --table '0 1 linear /dev/dm-1 0'
  ...

Peter - have I missed anything?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA

[Alasdair G Kergon]

On Tue, Jul 27, 2021 at 12:18:02PM +0200, Thore Sommer wrote:
> How is the measured uuid created? The format seems to be
> "CRYPT-VERITY-UUID-NAME" where UUID is uuid from the verity device and NAME is
> the device mapper name. Does this naming come from the kernel or 
> libcryptsetup?

See libdevmapper.h:
/*
 * Configure default UUID prefix string.
 * Conventionally this is a short capitalised prefix indicating the subsystem
 * that is managing the devices, e.g. "LVM-" or "MPATH-".
 * To support stacks of devices from different subsystems, recursive functions
 * stop recursing if they reach a device with a different prefix.
 */
int dm_set_uuid_prefix(const char *uuid_prefix);


Each device-mapper device may have a uuid of up to 128 characters plus
trailing NUL.  Whichever piece software activates the device assigns the
uuid (so userspace or kernel boot parameters).  By convention each such
piece of software uses a short prefix ending with a hyphen that
identifies that software as the "owner" (manager) of that dm device.
This means each piece of software can easily filter out the devices for
which it is responsible and ignore all the others etc.  It can use the
remainder of the UUID to identify the device uniquely to itself.
Another convention is that when one device is a 'wrapper' of some sort
around another, it may create the uuid by adding its prefix to the uuid
of the device it is wrapping - this might give you stacked prefixes.
When there's a complex one-composed-from-many device structure, suffices
may be used to identify the components.

Think of the 'name' as the human-friendly device name and the uuid as
a software-friendly internal name.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA

[Alasdair G Kergon]

On Tue, Jul 27, 2021 at 12:18:02PM +0200, Thore Sommer wrote:
> No new IMA measurement is generated if dm-verity verification fails. This is
> unfortunate because to make the dm-verity measurements useful we need to be
> notified when hash_failed is set to true. We will need something like
> "device_update" where we remeasure the device state if it has changed.
 
Measurements in the current patchset are only triggered by ioctl calls
initiated by userspace.

Having other triggering mechanisms - such as hooking into internal
events when something unexpected happens - could be considered for
follow-on patches.

> Creating a dm-verity device with mount then removing it and now if you create 
> it
> again no measurement is generated. Is that the expected behavior?  

Each of the relevant dm ioctls should be logged separately each time.  If that's
not happening it might need fixing.

> Is there a reason that suspend is not measured?

A suspend doesn't change the configuration so falls outside the criteria
for this patch set.  (If there is some need for it, it would be covered
by the need to measure internal events that I mentioned above.)
 
> What can happen in between a "table_load" and "device_resume" that is security
> relevant?

The table prepared by the load can be cleared.  That would change the
effect of the resume.
 
> Not directly device mapper related, but it would be nice to also measure the
> mount points and a mapping to the device IDs. 

Again, that would be for future patches building on these ones.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 6/7] dm: update target specific status functions to measure data

[Alasdair G Kergon]

On Mon, Jul 12, 2021 at 05:49:03PM -0700, Tushar Sugandhi wrote:
> The DM target data measured by IMA subsystem can alternatively
> be queried from userspace by setting DM_IMA_MEASUREMENT_FLAG with
> DM_TABLE_STATUS_CMD.

I was able to try this out - as 'dmsetup measure' - by applying the quick
patch below to the upstream LVM2 tree.
 
Alasdair


diff --git a/libdm/.exported_symbols.DM_1_02_179 
b/libdm/.exported_symbols.DM_1_02_179
new file mode 100644
index 0..4ab603b68
--- /dev/null
+++ b/libdm/.exported_symbols.DM_1_02_179
@@ -0,0 +1 @@
+dm_task_ima_measurement
diff --git a/libdm/dm-tools/dmsetup.c b/libdm/dm-tools/dmsetup.c
index a3d1248bc..3e5983fef 100644
--- a/libdm/dm-tools/dmsetup.c
+++ b/libdm/dm-tools/dmsetup.c
@@ -2446,6 +2446,9 @@ static int _status(CMD_ARGS)
if (_switches[NOFLUSH_ARG] && !dm_task_no_flush(dmt))
goto_out;
 
+   if (!dm_task_ima_measurement(dmt))
+   goto_out;
+
if (!_task_run(dmt))
goto_out;
 
@@ -6262,6 +6265,7 @@ static struct command _dmsetup_commands[] = {
{"stats", " [] [...]", 1, -1, 1, 1, _stats},
{"status", "[...] [--noflush] [--target ]", 0, -1, 
2, 0, _status},
{"table", "[...] [--concise] [--target ] 
[--showkeys]", 0, -1, 2, 0, _status},
+   {"measure", "[...]", 0, -1, 2, 0, _status},
{"wait", " [] [--noflush]", 0, 2, 0, 0, _wait},
{"mknodes", "[...]", 0, -1, 1, 0, _mknodes},
{"mangle", "[...]", 0, -1, 1, 0, _mangle},
diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c
index 47f14398c..22cce8e76 100644
--- a/libdm/ioctl/libdm-iface.c
+++ b/libdm/ioctl/libdm-iface.c
@@ -929,6 +929,13 @@ int dm_task_secure_data(struct dm_task *dmt)
return 1;
 }
 
+int dm_task_ima_measurement(struct dm_task *dmt)
+{
+   dmt->ima_measurement = 1;
+
+   return 1;
+}
+
 int dm_task_retry_remove(struct dm_task *dmt)
 {
dmt->retry_remove = 1;
@@ -1286,7 +1293,14 @@ static struct dm_ioctl *_flatten(struct dm_task *dmt, 
unsigned repeat_count)
}
dmi->flags |= DM_UUID_FLAG;
}
-
+   if (dmt->ima_measurement) {
+   if (_dm_version_minor < 45) {
+   log_error("WARNING: IMA measurement unsupported by "
+ "kernel.  Aborting operation.");
+   goto bad;
+   }
+   dmi->flags |= DM_IMA_MEASUREMENT_FLAG;
+   }
dmi->target_count = count;
dmi->event_nr = dmt->event_nr;
 
@@ -1487,6 +1501,7 @@ static int _create_and_load_v4(struct dm_task *dmt)
task->head = dmt->head;
task->tail = dmt->tail;
task->secure_data = dmt->secure_data;
+   task->ima_measurement = dmt->ima_measurement;
 
r = dm_task_run(task);
 
@@ -1875,7 +1890,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, 
unsigned command,
}
 
log_debug_activation("dm %s %s%s %s%s%s %s%.0d%s%.0d%s"
-"%s[ %s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] 
(*%u)",
+"%s[ %s%s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] 
(*%u)",
 _cmd_data_v4[dmt->type].name,
 dmt->new_uuid ? "UUID " : "",
 dmi->name, dmi->uuid, dmt->newname ? " " : "",
@@ -1893,6 +1908,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, 
unsigned command,
 dmt->retry_remove ? "retryremove " : "",
 dmt->deferred_remove ? "deferredremove " : "",
 dmt->secure_data ? "securedata " : "",
+dmt->ima_measurement ? "ima_measurement " : "",
 dmt->query_inactive_table ? "inactive " : "",
 dmt->enable_checks ? "enablechecks " : "",
 dmt->sector, _sanitise_message(dmt->message),
diff --git a/libdm/ioctl/libdm-targets.h b/libdm/ioctl/libdm-targets.h
index 294210d2b..022b02c72 100644
--- a/libdm/ioctl/libdm-targets.h
+++ b/libdm/ioctl/libdm-targets.h
@@ -69,6 +69,7 @@ struct dm_task {
int enable_checks;
int expected_errno;
int ioctl_errno;
+   int ima_measurement;
 
int record_timestamp;
 
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index ac31b59da..e9412da7d 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -235,6 +235,7 @@ int dm_task_suppress_identical_reload(struct dm_task *dmt);
 int dm_task_secure_data(struct dm_task *dmt);
 int dm_task_retry_remove(struct dm_task *dmt);
 int dm_task_deferred_remove(struct dm_task *dmt);
+int dm_task_ima_measurement(struct dm_task *dmt);
 
 /*
  * Record timestamp immediately after the ioctl returns.
diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index 708414676..d123e3ddf 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -336,6 +336,7 @@ struct dm_task 

Re: [dm-devel] dm thin-volume hung as swap: bug or as-design ?

[Alasdair G Kergon]

On Fri, Jan 29, 2021 at 06:40:06PM +0800, Coly Li wrote:
> Recently I receive a report that whole system hung and no response after
> a while with I/O load. The special configuration is the dm thin-pool
> volume is used as the swap partition of the system.
> My questions are,
> - Can a thin-pool volume be used as swap device?

Yes in principle, but it won't get much testing as it's not 
necessarily a particularly sensible configuration.
- You'd normally prefer fully-pre-allocated disk space for swap
  and turn off the zeroing.

Is there some use-case where it does make more sense?

> - The above description is a bug, or an already know issue which should
> be avoided ?
 
Bug.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] DM's filesystem lookup in dm_get_dev_t() [was: Re: linux-next: manual merge of the device-mapper tree with Linus' tree]

[Alasdair G Kergon]

On Tue, Dec 22, 2020 at 06:24:09PM +0100, Hannes Reinecke wrote:
> However, lookup_bdev() now always recurses into the filesystem, causing 
> multipath to stall in an all-paths-down scenario.
 
I have not read the background about whatever the new problem is - I'm
jumping in cold seeing this message - but from the very beginning of
device-mapper we have strongly recommended that userspace supplies the
block device in the form MAJOR:MINOR and all our own tools do that.  We
guarantee not to deadlock in these places when this is done.

We also accept the device in the form of a path name as we know there
are times when this is safe and convenient, but then we offer no
guarantees - we place the responsibility upon userspace only to do this
when it knows it is safe to do so i.e. no race and no deadlock.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/2] introduce interface to list all badblocks

[Alasdair G Kergon]

On Mon, Jun 08, 2020 at 04:45:11PM +0800, yangerkun wrote:
> $ sudo dmsetup message dust1 0 listbadblocks
> The following message will appear, listing one bad block number per

Did you consider returning the data directly to the caller so it
can be accessed directly?

(e.g. like @stats_list handled in dm-stats.c)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm: track io errors per mapped device

[Alasdair G Kergon]

On Thu, May 07, 2020 at 07:05:33PM -0400, Kjetil Orbekk wrote:
> This will track ioerr_cnt on all dm targets and expose it as
> /dm/ioerr_cnt.

How do you propose to use this?
What are you trying to measure and why?
- How exact must the number be to meet your requirements?

Or to put it another way, do you need to know the exact number you are
exposing, or do you derive something else from this which could also be
derived from an alternative number?

In particular, given the way we split and clone and stack devices (so
there may be an element of multiplication), and reload tables (so
existing values might become irrelevant), did you consider alternative
semantics before selecting this approach?
(Or to put it another way, is there a need to reset it or track
the value since the last resume?)

(Documentation is also needed - which ought to explain the semantics
and how the observed values interact with the use of device-mapper
features.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] Possible null pointer dereference in __rh_alloc()

[Alasdair G Kergon]

On Sun, May 03, 2020 at 03:02:21PM +0800, Dongyang Zhan wrote:
>  I am a security researcher, my name is Dongyang Zhan. I found a potential 
> bug in
>  /drivers/md/dm-region-hash.c in Linux 4.10.17. I hope you can help me to 
> confirm it.
>  __rh_alloc() in /drivers/md/dm-region-hash.c mishandles the memory 
> allocation failures of nreg.
>  Source code:
>  struct dm_region *reg, *nreg;
>  nreg = mempool_alloc(rh->region_pool, GFP_ATOMIC);
>  if (unlikely(!nreg))
>  nreg = kmalloc(sizeof(*nreg), GFP_NOIO | __GFP_NOFAIL);
>  nreg->state = rh->log->type->in_sync(rh->log, region, 1) ?
>  DM_RH_CLEAN : DM_RH_NOSYNC;
>  If the statement (nreg = kmalloc(sizeof(*nreg), GFP_NOIO | __GFP_NOFAIL);) 
> fails,
>  dereferencing this pointer (nreg->state) will cause null pointer dereference.

 * %__GFP_NOFAIL: The VM implementation _must_ retry infinitely: the caller
 * cannot handle allocation failures. The allocation could block
 * indefinitely but will never return with failure. Testing for
 * failure is pointless.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [linux-lvm] storage-logger: Recording changes to the udev database

[Alasdair G Kergon]

On Fri, Mar 27, 2020 at 03:50:06PM -0400, Brian McCullough wrote:
> In your instructions, you say to put the "working" script in /usr/sbin,
> while in the udev rule, it is in /sbin/

In Fedora they are the same.

> I tried adding and removing a USB-connected drive, but did not see any
> response in the journal.  Should I have seen something, or are those not
> seen?

You should test the script in isolation in case there are any other 
differences on Debian.  Change the top line to '...bash -x' so you
can watch it, set some environment variables (like ACTION=add) and 
run it directly.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] storage-logger: Recording changes to the udev database

[Alasdair G Kergon]

I'm experimenting with ways of recording changes to the udev database so
you can look back at the history of the storage stack on a particular
machine.  This is still a work-in-progress, but it's reached a point
where I'd like more people to try it out.

I've written a shell script that records data related to storage uevents
in the system journal and a perl script that helps you to interrogate
this data later to create a representation of the storage components.

If you're interested, please try this out and let me know if you think
pursing this approach further would lead to something that you would
use and distributions should ship.

Source code:
  https://github.com/lvmteam/storage-logger
Fedora builds:
  https://copr.fedorainfracloud.org/coprs/agk/storage-logger/build/1320735/
Presentation:
  https://fosdem.org/2020/schedule/event/storage_logger/


Storage-logger
==
The storage-logger project maintains a record of the storage
configuration of a linux system as it changes over time.
The idea is to provide a quick way to check the state
of a system at times in the past.

Logging
===
The initial logging implementation is triggered by storage uevents and
consists of two components:

1. A new udev rule file, 99-zzz-storage-logger.rules, which runs after
all the other rules have run and invokes:

2. A script, udev_storage_logger.sh, that captures relevant
information about devices that changed and stores it in the system
journal.

The effect is to log relevant uevents plus some supplementary
information.  It does not yet handle filesystem-related events.

Reporting
=
Two methods to query the data are offered:

1. journalctl
Reports the raw data using simple filtering.
Data is tagged with the identifier UDEVLOG and retrievable as
key-value pairs.
All the captured data:
  journalctl -t UDEVLOG --output verbose
or as JSON:
  journalctl -t UDEVLOG --output json
Between a time range:
  --since '-MM-DD HH:MM:SS' 
  --until '-MM-DD HH:MM:SS'
Other filtering features are described in the man page.

2. lsblkj
This wrapper creates a dummy system environment that "looks like" the
system did at a specified earlier time and then runs lsblk against it.
It accepts --since and --until arguments to pass to journalctl to
select the desired data, and passes other arguments controlling
the output format to the real lsblk.  
Use --verbose to watch it setting up the temporary environment .
Use --dry-run to see what it would do without actually doing it.
Use --git to create a git repository recording the changes over time.
 
Alasdair
-- 
a...@redhat.com

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] dm: expose dm_copy_name_and_uuid()

[Alasdair G Kergon]

On Fri, Feb 07, 2020 at 01:24:33AM +, Alasdair G Kergon wrote:
> In other words, NEVER report name/uuid without ALSO still reporting
> dm_device_name alongside it.

The reason we only log dm_device_name() is because it is the minimum 
necessary to uniquely identify the device and tie together all
the messages relating to it.

Adding name/uuid to every message would make log messages unduly long.
We could offer an in-kernel option to log all setting and changing
of device names and uuids in the dm core, though I might argue that that
would just be covering up inadaquate logging in the userspace tools
making the changes.

Storage-logger offers a compromise that records it all from the
generated uevents.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] dm: expose dm_copy_name_and_uuid()

[Alasdair G Kergon]

On Thu, Feb 06, 2020 at 07:28:04PM -0500, John Dorminy wrote:
> To be clear, I care more about getting the name in kernelspace than
> about getting the UUID in kernelspace. (253:0) is a unique name for

>From the kernel's point of view, it is THE unique and definitive name
for the lifetime of the device.  It's the one thing userspace can rely
upon (without assuming additional 'good' userspace behaviour) to tie
together messages relating to the same device instance.

> test-test, and we can get that via dm_device_name() at the moment. But
> test-test is often a better name -- it's the name the user provided

It can be changed and so processing can't rely on it without trusting
both the userspace management code's self-imposed constraints and the
sysadmin not to have worked around them.

In other words, NEVER report name/uuid without ALSO still reporting
dm_device_name alongside it.

Also take a look at
  https://github.com/lvmteam/storage-logger
which tackles logging all the relevant device information.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] linux-next: Tree for Nov 15 (drivers/md/dm-integrity)

[Alasdair G Kergon]

On Fri, Nov 15, 2019 at 08:19:53AM -0800, Randy Dunlap wrote:
> BTW, dm-devel@redhat.com seems to be invalid or dead.

Red Hat made a few changes to its mail configuration over the last
week.

If anyone reading this still has problems that might be related to
this, please send me the relevant details privately (e.g. full headers
of messages) and I'll try to engage the right people to help.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] bug: dmsetup fails to parse concise tables

[Alasdair G Kergon]

On Sat, Feb 23, 2019 at 10:56:59PM +0100, Kurt Garloff wrote:
> if I store device-mapper tables with dmsetup table --concise, I would
> expect that dmsetup create --concise would be able to parse the tables.

That was the intention.  Thanks for reporting the bug - fix applied upstream.
 
Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] bug: dmsetup fails to parse concise tables

[Alasdair G Kergon]

On Sat, Feb 23, 2019 at 11:01:45PM +0100, Kurt Garloff wrote:
> Fixing patch is attached. The parser not never get to the third entry ...

Try this.

Alasdair

--- a/tools/dmsetup.c
+++ b/tools/dmsetup.c
@@ -369,7 +369,7 @@ static int _parse_table_lines(struct dm_task *dmt)
 
do {
/* Identify and terminate each line */
-   if ((next_pos = strchr(_table, '\n')))
+   if ((next_pos = strchr(pos, '\n')))
*next_pos++ = '\0';
if (!_parse_line(dmt, pos, "", ++line))
return_0;

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [RFC] dm-bow working prototype

[Alasdair G Kergon]

On Wed, Oct 24, 2018 at 03:24:29PM -0400, Mikulas Patocka wrote:
> What about allocating a big file, using the FIEMAP ioctl to find the 

For reference, dmfilemapd in the lvm2 tree (in daemons/) uses FIEMAP
(in libdm/libdm-stats.c) for monitoring I/O by file.

> If you decide that rollback is no longer needed, you just unload the 
> snapshot target and delete the big file. If you decide that you want to 
> rollback, you can use the snapshot merge functionality (or you can write a 
> userspace utility that does offline merge).
 
There's some old code from Mark McLoughlin for userspace snapshot merging here:
  https://people.gnome.org/~markmc/code/merge-dm-snapshot.c

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [RFC] dm-bow working prototype

[Alasdair G Kergon]

On Tue, Oct 23, 2018 at 02:23:28PM -0700, Paul Lawrence wrote:
> It is planned to use this driver to enable restoration of a failed
> update attempt on Android devices using ext4.

Could you say a bit more about the reason for this new dm target so we
can understand better what parameters you are trying to optimise and
within what new constraints?  What are the failure modes that you need
to handle better by using this?  (We can guess some answers, but it
would better if you can lay them out so we don't need to make
assumptions.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] LVM snapshot broke between 4.14 and 4.16

[Alasdair G Kergon]

On Fri, Aug 03, 2018 at 01:42:52PM -0700, Linus Torvalds wrote:
> So even in case 2, we do try to avoid versioning. More often we add a
> new flag, and say "hey, if you want the new behavior, use the new flag
> to say so". Not versioning, but explicit "I want the new behavior"
 
There are spare flags available in dm-ioctl - one could indeed turn on 
the new more-restrictive behaviour.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] LVM snapshot broke between 4.14 and 4.16

[Alasdair G Kergon]

On Fri, Aug 03, 2018 at 08:56:36PM +0100, Alasdair G Kergon wrote:
> Anything passing in version 4.37.0 or earlier (which is the version in

If taking this approach, it might be better to use the current version
i.e.  where we add the kernel-side fix.  IOW anything compiling against
a uapi header taken from a kernel up to now will see the old behaviour,
but anything newer (after we next increment the kernel dm version) will
be required to change its userspace code if it has this problem in it.

The problematic versions of lvm2 ship with versions up to and including
4.36.0 in this field so should be caught either way.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] LVM snapshot broke between 4.14 and 4.16

[Alasdair G Kergon]

On Fri, Aug 03, 2018 at 11:20:34AM -0400, Theodore Y. Ts'o wrote:
> It *used* to be the case that users running RHEL 2 or RHEL 3 could try
> updating to the latest upstream kernel, and everything would break and
> fall apart.  This was universally considered to be a failure, and a
> Bad Thing.  So if LVM2 is not backwards compatible, and breaks in the
> face of newer kernels running older distributions, that is a bug.
 
Brings back memories!  For those who wonder what this is all about,
with LVM1, if the version of kernel and userspace didn't match it simply
stopped.  No booting into the previous version of your kernel after
upgrading unless you reverted userspace as well!  Led to all sorts of
not-so-fancy workarounds.

As a reaction to this, LVM2 (through libdevmapper and anything else
using the dm ioctls as documented in dm-ioctl.h) passes a version number
up to the kernel (to say "I know about kernels with dm up to and
including version x.y.z"), so there is an option for an in-kernel
workaround here to limit its compatibility mode to the broken userspace
versions.  

Anything passing in version 4.37.0 or earlier (which is the version in
dm-ioctl.h when this kernel patch was applied) could be assumed to
require the old behaviour.  check_version() is where this version is
seen, so it would either store it for later checking or do the check and
store a flag to invoke compatible behaviour later.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2 2/2] dm unstripe: Add documentation for unstripe target

[Alasdair G Kergon]

On Tue, Dec 12, 2017 at 07:45:56AM -0700, Keith Busch wrote:
> Ah, this device's makers call the "stripe" size what should be called
> "chunk".

If this target is to go anywhere, let's try to define it as 'undoing'
the existing dm-stripe target using primary terminology, field names
etc. as close as possible to our existing target.  We can still mention
any alternative terminology encountered in the documentation.  The
first (simplest) documented example could be a dm-stripe target being
'unstriped'.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm thin: fix NULL pointer exception caused by two concurrent "vgchange -ay -K " processes.

[Alasdair G Kergon]

On Fri, Nov 24, 2017 at 04:59:13PM +0800, monty_pa...@sina.com wrote:
> All that we need do is to ensure pool_target registering ops
> is the last ops in dm_thin_init.
 
Any other targets wrong too?  cache?  multipath?  snapshot?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 4/4] dm: convert table_device.count from atomic_t to refcount_t

[Alasdair G Kergon]

On Fri, Nov 24, 2017 at 08:29:42AM +, Reshetova, Elena wrote:
> By looking at the code, I don't see where the change in the reference counting
> could have caused this. 
 
The cause was the bug I identified in patch 3, not this patch.

The regression is easily hit - tables that reference the same underlying device
more than once are very common.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 3/4] dm: convert dm_dev_internal.count from atomic_t to refcount_t

[Alasdair G Kergon]

On Fri, Oct 20, 2017 at 10:37:38AM +0300, Elena Reshetova wrote:
>   } else if (dd->dm_dev->mode != (mode | dd->dm_dev->mode)) {
>   r = upgrade_mode(dd, mode, t->md);
>   if (r)
>   return r;
> + refcount_inc(>count);
>   }

Missing here:

else
refcount_inc(>count);

?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] dm-crypt: Reject sector_size feature if device length is not aligned to it

[Alasdair G Kergon]

On Tue, Oct 03, 2017 at 02:08:04PM -0400, Mike Snitzer wrote:
> Not sure why you're putting such value on that behaviour.  The earlier
> we catch invalid tables the better off we are.  Failing at resume time
> sucks (always has).
 
Validation code shouldn't be making assumptions about things that lie
completely outside its control and falsely failing operations that would
actually succeed if they were allowed to proceed.  The existing
kernel/userspace interface does not require userspace to load devices in
any particular sequence.  We could have provided a tree-based kernel/userspace
interface with stronger requirements like these, but the fact is, we haven't.
Perhaps we will one day.

As a minimum, you would need to change the patch to validate against the
inactive tables of underlying devices instead of the live ones - i.e. assume
that userspace already loaded all the underlying devices (and will resume them
all before the one being validated gets resumed).  Currently no such
ordering requirement is imposed on userspace, so you'd also need a
compatibility flag to enable the stronger contraints.

This patch can break valid userspace code.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm-crypt: Reject sector_size feature if device length is not aligned to it

[Alasdair G Kergon]

On Mon, Oct 02, 2017 at 10:43:56AM -0400, Mikulas Patocka wrote:
> Here I send a patch that checks invalid limits when the table is loaded 
> and aborts the table load ioctl with an error.
 
The code does not already do this because it needs to avoid unnecessary
failures if any characteristics of the devices underneath change after
the 'load' but before the 'resume' (as can happen, for example, when a
tree of devices gets updated by performing all the loads before all the
resumes).

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/3] dm: a basic support for using the select or poll function

[Alasdair G Kergon]

On Thu, May 11, 2017 at 09:30:43PM +0200, Martin Wilck wrote:
> I see - but I don't see yet how the ioctl approach (or the
> close()/open() based one, for that matter) would avoid this race.
>  1) application processes event N
>  2) event N+1 arrives in the kernel
>  3) user space issues ioctl or close()/open() sequence, N+1 is recorded
> in priv->global_event_nr
>  4) user space runs poll()
>  5) event N+2 arrives 4 weeks later and poll returns
 
Well that userspace design is obviously not compatible with the set of 
patches that has been posted and is not what was proposed in the patch
headers.

Userspace records and compares event numbers.

ARM occurs immediately before LIST.  Any userspace processing in response
happens later.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/3] dm: a basic support for using the select or poll function

[Alasdair G Kergon]

On Thu, May 11, 2017 at 10:45:27PM +0200, Martin Wilck wrote:
> Except that it can cause userspace to miss events, as I tried to point
> out in my reply to Mike.

It should never miss new events.  Because the calls are separate,
occasionally it might trigger when it didn't need to, but userspace will
see nothing new happened and just go back into poll.  Combining the two
calls would avoid this, but the authors decided on balance it was
unnecessary.

I've already requested a libdevmapper/dmsetup implementation as a matter
of urgency so that everyone has easy command-line access to the new
features and example code and can test them - normally patches like
these shouldn't go upstream until a supported userspace implementation
is available.  Once we have this, questions like the ones you pose can
be checked practically.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/3] dm: a basic support for using the select or poll function

[Alasdair G Kergon]

On Thu, May 11, 2017 at 09:49:14PM +0200, Martin Wilck wrote:
> Here is an idea: the best way to avoid the race mentioned by Mike might
> be to set priv->global_event_nr to the highest event nr reported by the
> DM_LIST_DEVICES ioctl when this ioctl returns. DM_LIST_DEVICES would
> then represent your "separate action", which would fit quite well, and
> the DM_DEV_ARM_POLL ioctl wouldn't be needed. It would be guaranteed
> that if any event had occured after the previous DM_LIST_DEVICES,
> whether or not that had been before or after the poll() call, userspace
> would notice.
 
It's not really good practice to overload operations by adding and
relying upon side-effects.  Yes, you could add a flag to control the
side-effect, but a separate ioctl seems to be a perfectly reasonable
approach in the present case, allowing for flexible use and not forcing
you to generate the list when you don't need it.  We've already got
examples of both other approaches - DM_DEV_WAIT_CMD taking the last
known event_nr as input, and @stats_print_clear reporting and resetting
counters in the same operation.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 08/10] kpartx: use partition UUID for non-DM devices

[Alasdair G Kergon]

On Sat, May 06, 2017 at 12:05:57AM +0200, Martin Wilck wrote:
> Introduce a "fake" UUID for these devices to make sure kpartx
> deletes only devices it had created previously. Otherwise kpartx
> might e.g. delete LVM LVs that are inside a device it is trying
> to delete partitions for. It seems to be wiser to make sure the
> user delete these manually before running "kpartx -d".
> 
> With the fake UUID in place, we can re-introduce the UUID check
> for non-DM device that was removed in the earlier patch
> "kpartx: dm_remove_partmaps: support non-dm devices".
> 
> This disables also deletion of partition mappings created
> by earlier versions of kpartx. If kpartx has been updated after
> partition mappings were created, the "-f" flag can be used
> to force delting these partitions, too.
 
Indeed - always supply a uuid for every dm device created.
Add a standard prefix to the beginning of it, ideally "KPARTX-" to be
consistent with other dm tools ("LVM-", "CRYPT-" etc.) if you're
able to break compatibility, but "part" is still unique and adequate.  
(Stacked devices can end up with a stack of prefixes - only the leftmost one
counts.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 00/10] fixes for kpartx -d

[Alasdair G Kergon]

On Sat, May 06, 2017 at 12:05:49AM +0200, Martin Wilck wrote:
>   3) kpartx should only delete "partitions", which are single-target
>  linear mappings into a block device. Other maps should not be touched.

The prefix on the dm device's uuid should guarantee this: all devices
kpartx creates should have the same initial characters (a
not-quite-standard form "part" IIRC instead of "KPARTX-") and any
devices without those initial characters must be ignored.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs

[Alasdair G Kergon]

Some more thoughts with your example, dmsetup might look like:

# dmsetup create --bootformat "lroot:uuid,rw,0 2097152 linear 8:2 0, \
2097152 2097152 linear 8:3 0, 4194304 2097152 linear 8:4 0"

- also supporting creating multiple devices if the semi-colon is used

- colon to separate name from uuid, like we already do major:minor 
- colon to separate other flags from rw if we need them in future

- splitting first on a unescaped semi-colons, then on the first two
unescaped commas, and then on unescaped commas and then unescaped spaces
within the table

- backslash escapes the following character so it is never a treated
as a separator
  - lroot\:uuid\;\\\ \"\, would be a device with no uuid and the name
lroot:uuid;\ ",  (on a non-udev system without name mangling)

# dmsetup ls --bootformat lroot
dm="lroot:uuid,rw,0 2097152 linear 8:2 0, 2097152 2097152 \
linear 8:3 0, 4194304 2097152 linear 8:4 0"

# dmsetup ls --bootformat
(all devices on one output line)

While the code also supports devices in the /dev/sda2 format for 
convenience, please use the preferred 8:2 format in any implementation
and documented examples (to avoid the unnecessary dependency on /dev and
its dependencies).

Or with some alternative name for the option
--boot[format|param]
--short[format]
--kernelparam
--condensed
other suggestions?

dmsetup create --condensed
dmsetup ls --condensed

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v6 0/3] dm: boot a mapped device without an initramfs

[Alasdair G Kergon]

On Thu, May 04, 2017 at 01:18:41PM +0200, Enric Balletbo Serra wrote:
> I'm wondering if a command line like this would be acceptable.

1) Make sure the implementation continues to support backslash quoting
so that any characters you introduce with special meanings (comma,
semi-colon, double-quote in that example) can still be used if required.

2) "none" is of course a valid uuid:)  More comma-separation or
re-ordering, perhaps?
 
3) Whatever final format is agreed here should be supported by dmsetup
as well, so you can both supply the format to dmsetup and ask dmsetup
to display your existing devices in this format.  Choose a format
that makes this easy.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm ioctl: prevent stack leak in dm ioctl call

[Alasdair G Kergon]

On Tue, Apr 25, 2017 at 05:33:19PM -0700, Adrian Salido wrote:
> Will update the patch to be clear
 
So at the moment, we're leaning towards just:

  param->data_size = offsetof(struct dm_ioctl, data)

to replace

  param->data_size = sizeof(*param);

in the caller.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm ioctl: prevent stack leak in dm ioctl call

[Alasdair G Kergon]

On Tue, Apr 25, 2017 at 05:33:19PM -0700, Adrian Salido wrote:
> it's actually the data portion of the struct under a custom user ioctl
> where (param_kernel->data_size - minimum_data_size) <
> sizeof(param_kernel->data)
> Will update the patch to be clear

Yes - but before updating the patch, we need to be clearer about the
requirements of the ioctl here.

Why are two different minimum data sizes used?

If we let userspace send a truncated dm_ioctl struct, why are we not
returning the same truncated one?

Is this the bug?
  param->data_size = sizeof(*param);

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm ioctl: prevent stack leak in dm ioctl call

[Alasdair G Kergon]

On Tue, Apr 25, 2017 at 04:31:29PM -0700, Adrian Salido wrote:
> Struct dm_ioctl has some padding/data that is not explicitly cleared
> before copying to user. This can cause kernel stack contents to be
> leaked to user space.

Please be more precise here, explaining which part of the buffer 
and under exactly what circumstances you have found that uninitialised 
content gets returned to userspace.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] New device mapper target ZDM

[Alasdair G Kergon]

Well to start with some device-mapper basics, looks at the newer files
(e.g.  thin-provisioning.txt, cache.txt) in the Documentation directory
to see what format to use and how to document the interface.
Constructor, status, examples using dmsetup only etc.

(BTW We don't use equals signs in our interfaces.)

But even before any of that, I'm puzzled by the basics of how this can
have been fitted into a device-mapper interface where the "sine qua non"
resume function is still TODO, 'dmsetup table' appears unimplemented,
'status' doesn't appear to provide the internal status, and 'load' has
some keywords that also don't appear to fit.

Look at how the existing dm targets divide up the work between each of
the functions and model your interface on those.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/9] Generate uevents for all DM events

[Alasdair G Kergon]

On Tue, Oct 04, 2016 at 04:39:28PM -0700, Andy Grover wrote:
> devicemapper is using uevents for:
> a. dm-verity detected corruption
> b. dm-multipath: path failed or reinstated
> c. dm device renamed
> d. there's also some use in md and bcache.
>
> devicemapper uses DM_EVENT ioctl (yuck) for:
> 1. dm-thin pool data/metadata filling up (hit a threshold)
> 2. dm-cache is now clean
> 3. dm-log flushed or log failed
> 4. dm-raid error detected or sync complete

> there doesn't seem to be much technical differentiation between the  
> two lists.

The distinction in dm is that events in the first category may affect
the availability of the device: they represent major (and hopefully
rare) changes.

Events in the second category are just notifications: no impact on /dev,
no need to trigger udev rules, and their use will continue to be
extended, and (rarely at the moment) could be frequent (which is no
problem for the existing polling-based mechanism).

> Instead of using uevent for everything, we could go to a separate  
> genetlink for 1-4 instead of making them use uevent like a-d, but we'd  
> end up with two different userspace notification techniques.

We see these as two different categories of notifications, and prefer
the greater flexibility a mechanism independent of uevents would
provide.  The team has discussed several alternatives over the years but
didn't make a decision as we've not yet reached a point where we're
straining the existing mechanism too far.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 6/6] libmultipath: fixup dm_rename to complete cookie on failure

[Alasdair G Kergon]

The library functions and return states are supposed to be well-defined.

If you think you've found a cookie leak on an error path within a library
function, we can investigate that and fix the library if need be.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 0/1] add option to output JSON for multipathd command

[Alasdair G Kergon]

On Wed, May 04, 2016 at 04:23:47PM -0400, Todd Gill wrote:
> I had earlier sent an email to dm-devel proposing we add a feature
> in multipathd to output multipath map topology in JSON format. This
> patch contains to the code for that feature.
> Having an option for the  CLI to output in JSON would allow higher
> level applications to more easily monitor/manage multipath.
 
Got any examples to show what the output actually looks like?

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 28/57] libmultipath: use a shared lock to co-operate with udev

[Alasdair G Kergon]

On Tue, May 03, 2016 at 09:27:32AM -0500, Benjamin Marzinski wrote:
> I don't see why this same issue can't happen for anyone creating any
> type of dm device build out of multiple physical devices, and AFAIK, lvm
> doesn't bother to protect users against this, even though it can now
> autoassemble. So I'm not against dropping this either.
 
LVM operations are co-ordinated using a variety of locks in
/var/lock/lvm.  The prefix "A_" is used for the locks designed to avoid
concurrent activation attempts on the same volume.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] dm-raid: check for zero feature flags in metadata

[Alasdair G Kergon]

On Fri, Apr 29, 2016 at 06:59:56PM +0200, hei...@redhat.com wrote:
> + rs->ti->error = "Unable to assemble array: No feature flags supported 
> yet";
> + if (le32_to_cpu(sb->features)) /* No features supported yet */

Is it worth splitting this into compat_flags and incompat_flags like we did in
thin and cache? 

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] MAINTAINERS: correct entry for LVM

[Alasdair G Kergon]

On Mon, Apr 11, 2016 at 09:45:01PM +0530, Sudip Mukherjee wrote:
> L stands for "Mailing list that is relevant to this area", and this is a  
> mailing list. :)

Your proposed patch isn't changing the L entry, so this is of no relevance.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH] MAINTAINERS: correct entry for LVM

[Alasdair G Kergon]

On Mon, Apr 11, 2016 at 08:50:39PM +0530, Sudip Mukherjee wrote:
> The entry of dm-devel@redhat.com was duplicated and the duplicate entry
> was marked as a Maintainer but it appears from the email address that it
> is a List. So remove the entry of M and only keep the L entry.
 
M and L are not mutually exclusive!

The definition of M is:
  M: Mail patches to: FullName 
and since we want patches to be sent to the mailing list, this entry is correct
as it stands.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel