diff mbox series

util: Extend get_root to find LUKS devices

Message ID 20210611100637.54475-1-stefano.babic@babic.homelinux.org
State Changes Requested
Headers show
Series util: Extend get_root to find LUKS devices | expand

Commit Message

Stefano Babic June 11, 2021, 10:06 a.m. UTC 
From: Stefano Babic <sbabic@denx.de>

This helps in case of encrypted filesystem or device mapper.
The returned device read from partitions is usually a dm-X device and
this does not show which is the block device that contains it. Look in
sysfs and check if the device has "slaves" entries, indicating the
presence of an underlying device. If found, return this instead of the
device returned parsing /proc/partitions.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 core/util.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Storm, Christian June 14, 2021, 8:43 a.m. UTC | #1 
Hi Stefano,

> +	snprintf(buf, sizeof(buf) - 1, "/sys/dev/block/%d:%d/slaves", dev_major, dev_minor);

This may fail, however, I guess it's safe here since the buffer is
strictly larger than the payload.

> +	n = scandir(buf, &devlist, filter_slave, NULL);
> +	if (n == 1) {
> +		devname = strdup(devlist[0]->d_name);
> +		free(devlist);
> +		return devname;
> +        }
> +	free(devlist);

This segfaulted at my side if devlist != NULL and continuing to parse /proc/partitions.
Initializing explicitly as
  struct dirent **devlist = NULL;
solved this.

By putting it in get_root_from_partitions(), BTRFS on dm-crypt isn't
recognized (due to its synthetic major:minor numbers), is this by intention?



Kind regards,
   Christian
Stefano Babic June 14, 2021, 10:34 a.m. UTC | #2 
Hi Christian,

On 14.06.21 10:43, Christian Storm wrote:
> Hi Stefano,
> 
>> +	snprintf(buf, sizeof(buf) - 1, "/sys/dev/block/%d:%d/slaves", dev_major, dev_minor);
> 
> This may fail, however, I guess it's safe here since the buffer is
> strictly larger than the payload.
> 
>> +	n = scandir(buf, &devlist, filter_slave, NULL);
>> +	if (n == 1) {
>> +		devname = strdup(devlist[0]->d_name);
>> +		free(devlist);
>> +		return devname;
>> +        }
>> +	free(devlist);
> 
> This segfaulted at my side if devlist != NULL and continuing to parse /proc/partitions.
> Initializing explicitly as
>    struct dirent **devlist = NULL;


Hi will add it in V2.

> solved this.
> 
> By putting it in get_root_from_partitions(), BTRFS on dm-crypt isn't
> recognized (due to its synthetic major:minor numbers), is this by intention?

I have not a system for easy testing - my test case was with an 
encrypted squashfs or an encrypted ext4. I would suggest you can send a 
follow up patch to take care of the btrfs case after this is merged. Is 
it ok for you ?

Best regards,
Stefano
Storm, Christian June 14, 2021, 10:53 a.m. UTC | #3 
Hi Stefano,

> > > +	snprintf(buf, sizeof(buf) - 1, "/sys/dev/block/%d:%d/slaves", dev_major, dev_minor);
> > 
> > This may fail, however, I guess it's safe here since the buffer is
> > strictly larger than the payload.
> > 
> > > +	n = scandir(buf, &devlist, filter_slave, NULL);
> > > +	if (n == 1) {
> > > +		devname = strdup(devlist[0]->d_name);
> > > +		free(devlist);
> > > +		return devname;
> > > +        }
> > > +	free(devlist);
> > 
> > This segfaulted at my side if devlist != NULL and continuing to parse /proc/partitions.
> > Initializing explicitly as
> >    struct dirent **devlist = NULL;
> 
> 
> Hi will add it in V2.
> 
> > solved this.
> > 
> > By putting it in get_root_from_partitions(), BTRFS on dm-crypt isn't
> > recognized (due to its synthetic major:minor numbers), is this by intention?
> 
> I have not a system for easy testing - my test case was with an encrypted
> squashfs or an encrypted ext4. I would suggest you can send a follow up patch
> to take care of the btrfs case after this is merged. Is it ok for you ?

Yes, sure.


Kind regards,
   Christian
diff mbox series

Patch

diff --git a/core/util.c b/core/util.c
index 51a16b6..f4cbbdf 100644
--- a/core/util.c
+++ b/core/util.c
@@ -24,6 +24,7 @@ 
 #include <libgen.h>
 #include <regex.h>
 #include <string.h>
+#include <dirent.h>
 
 #if defined(__linux__)
 #include <sys/statvfs.h>
@@ -851,6 +852,10 @@  size_t snescape(char *dst, size_t n, const char *src)
 /*
  * This returns the device name where rootfs is mounted
  */
+
+static int filter_slave(const struct dirent *ent) {
+	return (strcmp(ent->d_name, ".") && strcmp(ent->d_name, ".."));
+}
 static char *get_root_from_partitions(void)
 {
 	struct stat info;
@@ -858,11 +863,28 @@  static char *get_root_from_partitions(void)
 	char *devname = NULL;
 	unsigned long major, minor, nblocks;
 	char buf[256];
-	int ret;
+	int ret, dev_major, dev_minor, n;
+	struct dirent **devlist;
 
 	if (stat("/", &info) < 0)
 		return NULL;
 
+	dev_major = info.st_dev / 256;
+	dev_minor = info.st_dev % 256;
+
+	/*
+	 * Check if this is just a container, for example in case of LUKS
+	 * Search if the device has slaves pointing to another device
+	 */
+	snprintf(buf, sizeof(buf) - 1, "/sys/dev/block/%d:%d/slaves", dev_major, dev_minor);
+	n = scandir(buf, &devlist, filter_slave, NULL);
+	if (n == 1) {
+		devname = strdup(devlist[0]->d_name);
+		free(devlist);
+		return devname;
+        }
+	free(devlist);
+
 	fp = fopen("/proc/partitions", "r");
 	if (!fp)
 		return NULL;