| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BANLkTikJpjeoarMCEhaQ69g7UyM9-PqpVw@mail.gmail.com>
Date: Thu, 9 Jun 2011 21:56:45 -0500
From: Will Drewry <wad@...omium.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, Kay Sievers <kay.sievers@...y.org>,
Jens Axboe <jaxboe@...ionio.com>,
Namhyung Kim <namhyung@...il.com>,
Trond Myklebust <Trond.Myklebust@...app.com>
Subject: Re: [PATCH v2] init: add root=PARTUUID=UUID,PARTNROFF=%d support
On Thu, Jun 9, 2011 at 5:33 PM, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Tue, 7 Jun 2011 17:19:33 -0500
> Will Drewry <wad@...omium.org> wrote:
>
>> Expand root=PARTUUID=UUID syntax to support selecting a root partition
>> by integer offset from a known, unique partition. This approach
>> provides similar properties to specifying a device and partition number,
>> but using the UUID as the unique path prior to evaluating the offset.
>>
>> For example,
>> root=PARTUUID=99DE9194-FC15-4223-9192-FC243948F88B,PARTNROFF=1
>> selects the partition with UUID 99DE.. then select the next
>> partition.
>>
>> This change is motivated by a particular usecase in Chromium OS where
>> the bootloader can easily determine what partition it is on (by UUID)
>> but doesn't perform general partition table walking.
>>
>> That said, support for this model provides a direct mechanism for the
>> user to modify the root partition to boot without specifically needing
>> to extract each UUID or update the bootloader explicitly when the root
>> partition UUID is changed (if it is recreated to be larger, for
>> instance). Pinning to a /boot-style partition UUID allows the arbitrary
>> root partition reconfiguration/modifications with slightly less
>> ambiguity than just [dev][partition] and less stringency than the
>> specific root partition UUID.
>>
>> At present, PARTNROFF is tied to PARTUUID use and must follow
>> immediately. It is possible to expand support to other values of root=
>> or to add other key-value attributes and orderings, but it seems
>> unnecessary to add the infrastructure without a concrete need.
>>
>> ...
>>
>> init/do_mounts.c | 34 ++++++++++++++++++++++++++++++----
>> 1 files changed, 30 insertions(+), 4 deletions(-)
>
> I have bad news.
>
> akpm:/usr/src/linux-3.0-rc2> grep -rl 'root=' Documentation
> Documentation/scsi/sym53c8xx_2.txt
> Documentation/scsi/ncr53c8xx.txt
> Documentation/kernel-parameters.txt
> Documentation/virtual/uml/UserModeLinux-HOWTO.txt
> Documentation/virtual/lguest/lguest.txt
> Documentation/filesystems/nfs/nfsroot.txt
> Documentation/filesystems/affs.txt
> Documentation/filesystems/ubifs.txt
> Documentation/m68k/kernel-options.txt
> Documentation/power/swsusp-dmcrypt.txt
> Documentation/early-userspace/README
> Documentation/kdump/kdump.txt
> Documentation/devicetree/booting-without-of.txt
> Documentation/security/Smack.txt
> Documentation/md.txt
> Documentation/ia64/xen.txt
> Documentation/intel_txt.txt
> Documentation/initrd.txt
> Documentation/x86/boot.txt
> Documentation/sound/oss/mwave
> Documentation/arm/SA1100/Assabet
> Documentation/frv/booting.txt
> Documentation/networking/cs89x0.txt
> Documentation/networking/cxgb.txt
> Documentation/init.txt
>
> Please work out which Documentation files need updating?
I wouldn't say that it's bad news - I'll track down the place that
seems to make the most sense.
>> index c0851a8..35329b2 100644
>> --- a/init/do_mounts.c
>> +++ b/init/do_mounts.c
>> @@ -85,12 +85,15 @@ no_match:
>>
>> /**
>> * devt_from_partuuid - looks up the dev_t of a partition by its UUID
>> - * @uuid: 36 byte char array containing a hex ascii UUID
>> + * @uuid: min 36 byte char array containing a hex ascii UUID
>> *
>> * The function will return the first partition which contains a matching
>> * UUID value in its partition_meta_info struct. This does not search
>> * by filesystem UUIDs.
>> *
>> + * If @uuid is followed by a ",PARTNROFF=%d", then the number will be
>> + * extracted and used as an offset from the partition identified by the UUID.
>> + *
>> * Returns the matching dev_t on success or 0 on failure.
>> */
>> static dev_t devt_from_partuuid(char *uuid_str)
>> @@ -98,6 +101,16 @@ static dev_t devt_from_partuuid(char *uuid_str)
>> dev_t res = 0;
>> struct device *dev = NULL;
>> u8 uuid[16];
>> + struct gendisk *disk;
>> + struct hd_struct *part;
>> + int offset = 0;
>> +
>> + if (strlen(uuid_str) < 36)
>> + goto done;
>> +
>> + /* Check for optional partition number offset attributes. */
>> + if (uuid_str[36])
>> + sscanf(&uuid_str[36], ",PARTNROFF=%d", &offset);
>
> Syntax errors here will be silently ignored. I expect that's common in
> this area of code, but please take a look, see if we should do
> something to help the poor user.
Sounds good. I can see a few ways to bubble up useful information and
still trigger the classic error path (potential partitions) too, I
think.
>> @@ -143,8 +171,6 @@ dev_t name_to_dev_t(char *name)
>> #ifdef CONFIG_BLOCK
>> if (strncmp(name, "PARTUUID=", 9) == 0) {
>> name += 9;
>> - if (strlen(name) != 36)
>> - goto fail;
>> res = devt_from_partuuid(name);
>> if (!res)
>> goto fail;
>
> The code changes the behaviour of name_to_dev_t(), so we should
> consider all callers. That's hibernation, block2mtd and md. Have you
> thought about (or perhaps tested) these code paths? Does it make sense
> to use the extended syntax in these cases? Is it possible to do so?
> Does it work? etc.
Good question(s) - given that name_to_dev_t is init-internal, I had
forgotten there were still a fair number of consumers. I'll review
the code paths in the places where I can't test them outright and make
a note and/or appropriate changes.
Thanks for the feedback!
will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists