Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 20:20 -0700, Richard Narron wrote:
> Under the /block/partitions directory the c programs have about 13 uses
> of memcmp() and 6 uses of strcmp().
Nearly all of the memcmp uses with strings kernel wide use
the equivalent of memcmp(foo, "bar", strlen("bar"));
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 20:20 -0700, Richard Narron wrote:
> Under the /block/partitions directory the c programs have about 13 uses
> of memcmp() and 6 uses of strcmp().
Nearly all of the memcmp uses with strings kernel wide use
the equivalent of memcmp(foo, "bar", strlen("bar"));
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: (please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0" Thanks for looking at this code. I do appreciate it. How about saving a byte and doing this instead? if (memcmp(flavour, "bsd", 4) == 0) I do appreciate your input as coding style is important, but so too is reliability. I don't trust the string functions and probably never will. It is not surprising to me that things like SQL injection and any number of other C string exploits are very common. IBM gave up on the idea of marking memory to keep track of data length with the 1401 machines in the 1950's. But Digital Equipment kept the idea alive of using null characters for a long time. Sadly the C programming language copied this bad idea for strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect. Under the /block/partitions directory the c programs have about 13 uses of memcmp() and 6 uses of strcmp().
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: (please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0" Thanks for looking at this code. I do appreciate it. How about saving a byte and doing this instead? if (memcmp(flavour, "bsd", 4) == 0) I do appreciate your input as coding style is important, but so too is reliability. I don't trust the string functions and probably never will. It is not surprising to me that things like SQL injection and any number of other C string exploits are very common. IBM gave up on the idea of marking memory to keep track of data length with the 1401 machines in the 1950's. But Digital Equipment kept the idea alive of using null characters for a long time. Sadly the C programming language copied this bad idea for strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect. Under the /block/partitions directory the c programs have about 13 uses of memcmp() and 6 uses of strcmp().
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
(please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: > On Fri, 26 May 2017, Joe Perches wrote: > > On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: > > > On Fri, 26 May 2017, Joe Perches wrote: > > > > On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > > > > > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > > > > > and NetBSD partitions and does a reasonable job picking out OpenBSD > > > > > and NetBSD UFS subpartitions. > > > > > > > > > > But for FreeBSD the subpartitions are always "bad". > > > > > > > > > > Kernel: > > > > > > > [] > > > > > block/partitions/msdos.c | 2 ++ > > > > > > > > [] > > > > > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > > > > > continue; > > > > > bsd_start = le32_to_cpu(p->p_offset); > > > > > bsd_size = le32_to_cpu(p->p_size); > > > > > + if (memcmp(flavour, "bsd\0", 4) == 0) > > > > > > > > Weird code. Why not: > > > > > > > > if (strcmp(flavor, "bsd") == 0) > > > > > > > > > > I instinctively trust the memcmp function as it seems more like > > > assembly language to me and more straight forward and more reliable than > > > strcmp. > > > > That really doesn't matter. > > > > Your code stores "bsd\0\0" and not just "bsd\0" > > > > Thanks for looking at this code. I do appreciate it. > > How about saving a byte and doing this instead? > >if (memcmp(flavour, "bsd", 4) == 0) > > I do appreciate your input as coding style is important, but so too is > reliability. > > I don't trust the string functions and probably never will. > > It is not surprising to me that things like SQL injection and any number of > other > C string exploits are very common. > > IBM gave up on the idea of marking memory to keep track of data length with > the 1401 machines in the 1950's. > > But Digital Equipment kept the idea alive of using null characters for a > long time. Sadly the C programming language copied this bad idea for > strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect.
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
(please keep replies on the list) On Fri, 2017-05-26 at 18:33 -0700, Richard Narron wrote: > On Fri, 26 May 2017, Joe Perches wrote: > > On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: > > > On Fri, 26 May 2017, Joe Perches wrote: > > > > On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > > > > > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > > > > > and NetBSD partitions and does a reasonable job picking out OpenBSD > > > > > and NetBSD UFS subpartitions. > > > > > > > > > > But for FreeBSD the subpartitions are always "bad". > > > > > > > > > > Kernel: > > > > > > > [] > > > > > block/partitions/msdos.c | 2 ++ > > > > > > > > [] > > > > > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > > > > > continue; > > > > > bsd_start = le32_to_cpu(p->p_offset); > > > > > bsd_size = le32_to_cpu(p->p_size); > > > > > + if (memcmp(flavour, "bsd\0", 4) == 0) > > > > > > > > Weird code. Why not: > > > > > > > > if (strcmp(flavor, "bsd") == 0) > > > > > > > > > > I instinctively trust the memcmp function as it seems more like > > > assembly language to me and more straight forward and more reliable than > > > strcmp. > > > > That really doesn't matter. > > > > Your code stores "bsd\0\0" and not just "bsd\0" > > > > Thanks for looking at this code. I do appreciate it. > > How about saving a byte and doing this instead? > >if (memcmp(flavour, "bsd", 4) == 0) > > I do appreciate your input as coding style is important, but so too is > reliability. > > I don't trust the string functions and probably never will. > > It is not surprising to me that things like SQL injection and any number of > other > C string exploits are very common. > > IBM gave up on the idea of marking memory to keep track of data length with > the 1401 machines in the 1950's. > > But Digital Equipment kept the idea alive of using null characters for a > long time. Sadly the C programming language copied this bad idea for > strings. Let's not argue the language. Please use what's normal for the language as that is readers of the code typically expect.
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. I'm new to this forum and did not know who you were so I looked up your name in old threads and oddly enough found this thread about a strcmp bug: http://marc.info/?l=linux-kernel=125848558316468=2 http://marc.info/?l=linux-kernel=125847802903422=2
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 26 May 2017, Joe Perches wrote: On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD and NetBSD partitions and does a reasonable job picking out OpenBSD and NetBSD UFS subpartitions. But for FreeBSD the subpartitions are always "bad". Kernel: [] block/partitions/msdos.c | 2 ++ [] @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part continue; bsd_start = le32_to_cpu(p->p_offset); bsd_size = le32_to_cpu(p->p_size); + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0) I instinctively trust the memcmp function as it seems more like assembly language to me and more straight forward and more reliable than strcmp. I'm new to this forum and did not know who you were so I looked up your name in old threads and oddly enough found this thread about a strcmp bug: http://marc.info/?l=linux-kernel=125848558316468=2 http://marc.info/?l=linux-kernel=125847802903422=2
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: > On Fri, 26 May 2017, Joe Perches wrote: > > > On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > > > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > > > and NetBSD partitions and does a reasonable job picking out OpenBSD > > > and NetBSD UFS subpartitions. > > > > > > But for FreeBSD the subpartitions are always "bad". > > > > > > Kernel: > > > [] > > > block/partitions/msdos.c | 2 ++ > > > > [] > > > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > > > continue; > > > bsd_start = le32_to_cpu(p->p_offset); > > > bsd_size = le32_to_cpu(p->p_size); > > > + if (memcmp(flavour, "bsd\0", 4) == 0) > > > > Weird code. Why not: > > > > if (strcmp(flavor, "bsd") == 0) > > > > I instinctively trust the memcmp function as it seems more like > assembly language to me and more straight forward and more reliable than > strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0"
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 16:30 -0700, Richard Narron wrote: > On Fri, 26 May 2017, Joe Perches wrote: > > > On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > > > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > > > and NetBSD partitions and does a reasonable job picking out OpenBSD > > > and NetBSD UFS subpartitions. > > > > > > But for FreeBSD the subpartitions are always "bad". > > > > > > Kernel: > > > [] > > > block/partitions/msdos.c | 2 ++ > > > > [] > > > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > > > continue; > > > bsd_start = le32_to_cpu(p->p_offset); > > > bsd_size = le32_to_cpu(p->p_size); > > > + if (memcmp(flavour, "bsd\0", 4) == 0) > > > > Weird code. Why not: > > > > if (strcmp(flavor, "bsd") == 0) > > > > I instinctively trust the memcmp function as it seems more like > assembly language to me and more straight forward and more reliable than > strcmp. That really doesn't matter. Your code stores "bsd\0\0" and not just "bsd\0"
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > and NetBSD partitions and does a reasonable job picking out OpenBSD > and NetBSD UFS subpartitions. > > But for FreeBSD the subpartitions are always "bad". > > Kernel:block/partitions/msdos.c | 2 ++ [] > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > continue; > bsd_start = le32_to_cpu(p->p_offset); > bsd_size = le32_to_cpu(p->p_size); > + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0)
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On Fri, 2017-05-26 at 03:48 -0700, Richard Narron wrote: > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > and NetBSD partitions and does a reasonable job picking out OpenBSD > and NetBSD UFS subpartitions. > > But for FreeBSD the subpartitions are always "bad". > > Kernel:block/partitions/msdos.c | 2 ++ [] > @@ -300,6 +300,8 @@ static void parse_bsd(struct parsed_part > continue; > bsd_start = le32_to_cpu(p->p_offset); > bsd_size = le32_to_cpu(p->p_size); > + if (memcmp(flavour, "bsd\0", 4) == 0) Weird code. Why not: if (strcmp(flavor, "bsd") == 0)
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On 05/26/2017 04:48 AM, Richard Narron wrote: > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > and NetBSD partitions and does a reasonable job picking out OpenBSD > and NetBSD UFS subpartitions. > > But for FreeBSD the subpartitions are always "bad". > > Kernel: > Though all 3 of these BSD systems use UFS as a file system, only > FreeBSD uses relative start addresses in the subpartition > declarations. > > The following patch fixes this for FreeBSD partitions and leaves > the code for OpenBSD and NetBSD intact: I queued this up 3 days ago, and replied as such. There's no need to keep sending it. -- Jens Axboe
Re: [PATCH v3 1/1] partitions/msdos: FreeBSD UFS2 file systems are not recognized
On 05/26/2017 04:48 AM, Richard Narron wrote: > The code in block/partitions/msdos.c recognizes FreeBSD, OpenBSD > and NetBSD partitions and does a reasonable job picking out OpenBSD > and NetBSD UFS subpartitions. > > But for FreeBSD the subpartitions are always "bad". > > Kernel: > Though all 3 of these BSD systems use UFS as a file system, only > FreeBSD uses relative start addresses in the subpartition > declarations. > > The following patch fixes this for FreeBSD partitions and leaves > the code for OpenBSD and NetBSD intact: I queued this up 3 days ago, and replied as such. There's no need to keep sending it. -- Jens Axboe
