Merge ~mfo/casper:lp1892369 into casper:main
Status: | Merged |
---|---|
Merged at revision: | 0208d5f99950947bbbb3483b5c311590c59dc106 |
Proposed branch: | ~mfo/casper:lp1892369 |
Merge into: | casper:main |
Diff against target: |
45 lines (+14/-3) 2 files modified
casper-md5check/casper-md5check.c (+7/-3) debian/changelog (+7/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ubuntu Installer Team | Pending | ||
Review via email: mp+417120@code.qastaging.launchpad.net |
Description of the change
casper-md5check fails to detect 'fsck.mode=skip' if it's the last option in /proc/cmdline; fix that.
Tests
=====
Before:
---
$ sudo apt install --yes casper
$ dpkg -s casper | grep Version:
Version: 1.467
1) Without fsck.mode=skip: check occurs (OK)
$ cat /proc/cmdline
BOOT_IMAGE=
$ sudo /usr/lib/
.
Checking integrity, this may take some time
^C
2) With fsck.mode=skip last: check occurs (WRONG!)
$ echo "$(cat /proc/cmdline) fsck.mode=skip" >/tmp/cmdline
$ sudo mount --bind /tmp/cmdline /proc/cmdline
$ cat /proc/cmdline
BOOT_IMAGE=
$ sudo /usr/lib/
.
Checking integrity, this may take some time
^C
$ sudo umount /proc/cmdline
3) With fsck.mode=skip before last: check skipped (OK; workaround available)
$ echo "$(cat /proc/cmdline) fsck.mode=skip workaround" >/tmp/cmdline
$ sudo mount --bind /tmp/cmdline /proc/cmdline
$ cat /proc/cmdline
BOOT_IMAGE=
ubuntu@jammy:~$ sudo /usr/lib/
.
Check skipped.
$ sudo umount /proc/cmdline
After:
---
$ sudo dpkg -i casper_
1) Without fsck.mode=skip: check occurs (OK), and hint is provided
$ sudo umount /proc/cmdline
$ cat /proc/cmdline
BOOT_IMAGE=
$ sudo /usr/lib/
.
Checking integrity, this may take some time (or try: fsck.mode=skip)
^C
2) With fsck.mode=skip last: check skipped (OK; FIXED!)
$ echo "$(cat /proc/cmdline) fsck.mode=skip" >/tmp/cmdline
$ sudo mount --bind /tmp/cmdline /proc/cmdline
$ cat /proc/cmdline
BOOT_IMAGE=
$ sudo /usr/lib/
.
Check skipped.
$ sudo umount /proc/cmdline
3) With fsck.mode=skip before last: check skipped (OK; no regression)
$ echo "$(cat /proc/cmdline) fsck.mode=skip workaround" >/tmp/cmdline
$ sudo mount --bind /tmp/cmdline /proc/cmdline
$ cat /proc/cmdline
BOOT_IMAGE=
ubuntu@jammy:~$ sudo /usr/lib/
.
Check skipped.
Details:
=======
Note it ends with '\n'.
$ cat /proc/cmdline
BOOT_IMAGE=
$ hexdump -C /proc/cmdline
00000000 42 4f 4f 54 5f 49 4d 41 47 45 3d 2f 76 6d 6c 69 |BOOT_IMAGE=/vmli|
<...>
00000060 3d 6f 66 66 20 76 74 2e 68 61 6e 64 6f 66 66 3d |=off vt.handoff=|
00000070 37 0a |7.|
00000072
Test-case from original source,
added 3 printf()s for debugging.
@ cmdline.c
#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
void parse_cmdline(void) {
FILE *cmdline = fopen("
char buf[1024];
char *bufp = buf, *tok;
char *theme;
if (!cmdline)
return;
fread(buf, 1023, 1, cmdline);
buf[strlen(buf)] = '\0';
printf(
while ((tok = strsep(&bufp, " ")) != NULL) {
printf("tok %p, bufp %p, string '%s'\n", tok, bufp, tok);
if (strncmp(tok, "vt.handoff=7", sizeof(
}
fclose(cmdline);
}
int main(void) {
parse_cmdline();
return 0;
}
$ gcc -o cmdline cmdline.c
Issue #1: strlen() gets wrong length
---
$ ./cmdline
strlen(buf) 118
tok 0x7fffa3864b60, bufp 0x7fffa3864b86, string 'BOOT_IMAGE=
tok 0x7fffa3864b86, bufp 0x7fffa3864ba5, string 'root=/
tok 0x7fffa3864ba5, bufp 0x7fffa3864ba8, string 'ro'
tok 0x7fffa3864ba8, bufp 0x7fffa3864bae, string 'quiet'
tok 0x7fffa3864bae, bufp 0x7fffa3864bb5, string 'splash'
tok 0x7fffa3864bb5, bufp 0x7fffa3864bc5, string 'mitigations=off'
tok 0x7fffa3864bc5, bufp (nil), string 'vt.handoff=7
>S�'
Issue #1: 0x72 == 114, strlen(buf) == 118, so '\0' allowed 3 extra chars in the buf (junk)
Fix #1: use memset instead.
$ ./cmdline
strlen(buf) 114
tok 0x7fffa7ed5900, bufp 0x7fffa7ed5926, string 'BOOT_IMAGE=
tok 0x7fffa7ed5926, bufp 0x7fffa7ed5945, string 'root=/
tok 0x7fffa7ed5945, bufp 0x7fffa7ed5948, string 'ro'
tok 0x7fffa7ed5948, bufp 0x7fffa7ed594e, string 'quiet'
tok 0x7fffa7ed594e, bufp 0x7fffa7ed5955, string 'splash'
tok 0x7fffa7ed5955, bufp 0x7fffa7ed5965, string 'mitigations=off'
tok 0x7fffa7ed5965, bufp (nil), string 'vt.handoff=7
'
See strlen(buf) is 114 now.
Issue #2: newline in last parameter
---
Notice a newline in last line above, and no match found.
Since strsep() didn't find a delimiter after the last token,
it doesn't overwrite that delimiter with '\0' thus strncmp()
doesn't match because sizeof("string") counts that '\0' char.
Fix #2: add '\n' as another delimiter byte in the string
$ ./cmdline
tok 0x7ffeed9d04b0, bufp 0x7ffeed9d04d6, string 'BOOT_IMAGE=
tok 0x7ffeed9d04d6, bufp 0x7ffeed9d04f5, string 'root=/
tok 0x7ffeed9d04f5, bufp 0x7ffeed9d04f8, string 'ro'
tok 0x7ffeed9d04f8, bufp 0x7ffeed9d04fe, string 'quiet'
tok 0x7ffeed9d04fe, bufp 0x7ffeed9d0505, string 'splash'
tok 0x7ffeed9d0505, bufp 0x7ffeed9d0515, string 'mitigations=off'
tok 0x7ffeed9d0515, bufp 0x7ffeed9d0522, string 'vt.handoff=7'
Match found
tok 0x7ffeed9d0522, bufp (nil), string ''
Issue #3: another loop iteration after the last token
---
It happens because we don't check for the token not found case
in strsep() (see manpage) which sets bufp to NULL.
In our case shouldnt be harmful, but for correctness, let's fix it.
Fix #3: '&& bufp != NULL' to the conditional
$ ./cmdline
tok 0x7ffce3426750, bufp 0x7ffce3426776, string 'BOOT_IMAGE=
tok 0x7ffce3426776, bufp 0x7ffce3426795, string 'root=/
tok 0x7ffce3426795, bufp 0x7ffce3426798, string 'ro'
tok 0x7ffce3426798, bufp 0x7ffce342679e, string 'quiet'
tok 0x7ffce342679e, bufp 0x7ffce34267a5, string 'splash'
tok 0x7ffce34267a5, bufp 0x7ffce34267b5, string 'mitigations=off'
tok 0x7ffce34267b5, bufp 0x7ffce34267c2, string 'vt.handoff=7'
Match found
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
I've merged and uploaded this, thanks for the detailed testing!