Re: Patch Review Process?

[Greg KH]

On Sat, Sep 04, 2021 at 07:45:19PM -0500, Andrew D Adamson wrote:
> I've seen lots of blogs, keynotes, and docs on the process for
> submitting patches, but I can't really find a write up on how the
> review process works. Is there a best practice for finding patches
> pending review other than watching the all mailing lists? I saw the
> patchwork.kernel.org server can track patches with 'action pending' but
> I'm not sure if that means it needs review or a maintainer to accept it.

Watch the mailing list for most subsystems.  For others, they use
patchwork, but it all depends on the subsystem.  Ask on the
subsystem-specific list if you want to know what they use.

hope this helps,

greg k-h

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re[2]: Kernel bug tracker

[Adverg Ebashinskii]

 
Hi Thomas,
 
Thanks for the brief explanation of the bugs.
 
>  i'll hand over my patch as guideline, or as 
>  base for own work, or just for review, testing, and posting
 
If you could share your patch here to understand the problem better I would 
gladly dig into it.
 
--
Regards,
Adverg Ebashinskii
 
 ___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re[6]: Kernel bug tracker

[Adverg Ebashinskii]

Hi Valdis,
 
> You might want to read this
 
Thanks for the info it was very interesting to read. My primary intention of 
getting into the Linux Kernel Development is that I myself used to be a C/Linux 
developer for years on the user side and pretty well-versed in the Linux 
user-space API. But the thing is that I have almost zero knowledge of what 
actually going on under the hood and how to debug and fix complicated problems 
related to the Kernel itself.  So this is the reason of my interest precisely 
to core subsystems.
 
--
Regards,
Adverg Ebashinskii
 
 ___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel bug tracker

[Thomas Schmitt]

Hi,

maybe i should not have pasted my patches into a new mail.
My mail client shows the first mail as three mails. Possibly an effect
of the mailbox-like format which it got by pasting in two send-ready
git patches.
Strangely it shows the second mail with the Rock Ridge patch as a
single one.

Sorry for any confusion in the receiving mail boxes.


Have a nice day :)

Thomas


___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Kernel bug tracker

[Thomas Schmitt]

Hi,

second patch proposal for isofs because Adverg Ebashinskii wrote:
> If you could share your patch here to understand the problem better I would
> gladly dig into it.


-cover-letter.patch

From 3d484405f0ad8d10ef490281da157bfdd7450cb6 Mon Sep 17 00:00:00 2001
From: Thomas Schmitt 
Date: Tue, 22 Sep 2020 12:35:52 +0200
Subject: [PATCH 0/1] isofs: truncate oversized Rock Ridge names to 255 bytes

Currently Rock Ridge names of length >= 254 are coarsely truncated by
discarding the whole NM entry where the overflow happened. This yields
name lengths of much less than the permissible 255 bytes.
There is no reason to see why to exclude length 254 and 255 and especially
to truncate by possibly a hundred or more bytes than necessary.

So i propose to raise the length of permissible names to 255 and to let
truncation yield exactly a string length of 255 bytes. Truncation shall
take care to invalidate UTF-8 debris at the end of the resulting string
(sorry ISO-8859).

---
Tests made:

Create an ISO 9660 image with file names of length 255, using file
/bin/true as input for both files:

  victim1=12345678901234567890123456789012345678901234567890
  victim1="$victim1"12345678901234567890123456789012345678901234567890
  victim1="$victim1"12345678901234567890123456789012345678901234567890
  victim1="$victim1"12345678901234567890123456789012345678901234567890
  victim1="$victim1"12345678901234567890123456789012345678901234567890
  victim1="$victim1"1234E
  victim2=ä
  victim2="$victim2"ä
  victim2="$victim2"ä
  victim2="$victim2"ä
  victim2="$victim2"ä
  victim2="$victim2"xää
  xorriso -outdev /tmp/test_rr_name.iso \
  -blank as_needed \
  -map /bin/true /"$victim1" \
  -map /bin/true /"$victim2"

Currently the names get truncated to byte lengths 93 and 95:

  mount /tmp/test_rr_name.iso /mnt/iso
  /bin/ls /mnt/iso

yields in xterm with bash

   12345678901234567890...60.more.bytes...1234567890123
  'ää'$'\303'

Note the leading blank with the plain ASCII name and the shell characters
with the name that has 2-byte UTF-8 characters.
But

  /bin/ls /mnt/iso | cat

yields

  12345678901234567890...60.more.bytes...1234567890123
  ää�

The extra characters in xterm seem to be triggered by the presence of the
half UTF-8 'ä' at the end. Its byte 0xc3 is there, byte 0xa4 is missing.
(xterm and /bin/ls are from Debian 10.)
If i make the UTF-8 name shorter to avoid truncation or if i move the 'x'
to the start to cause truncation between complete UTF-8 'ä', the extra
characters do not show up in ls to xterm.

After my change in fs/isofs i get from /bin/ls /mnt/iso

  1234567890...230.more.bytes...12345678901234E
  ää...210.more.bytes...ääxää

Both strings have 255 bytes.

xorriso cannot be talked into writing longer Rock Ridge names. So i rather
set the new macro RR_NAME_LEN in rock.h to 33 to force truncation.
The result with /bin/ls -1 /mnt/iso is:

  123456789012345678901234567890123
  _

Note the half 'ä' at the end being mapped to '_'.
So all characters are valid UTF-8 and no oddities of ls or xterm are to
see.

---
Remaining checkpatch.pl warning:

scripts/checkpatch.pl complains about the string
  'ää'$'\303'
in this text by:
  WARNING: Possible unwrapped commit description (prefer a maximum 75
  chars per line)

Maybe it should talk about "bytes" rather than "chars" or learn about
multi-byte characters in UTF-8.

I think it is beneficial if i show the whole mangled name, rather than
describing it by some ASCII-only text.

---

Have a nice day :)

Thomas

Thomas Schmitt (1):
  isofs: truncate oversized Rock Ridge names to 255 bytes

 fs/isofs/rock.c | 73 ++---
 fs/isofs/rock.h |  2 ++
 2 files changed, 71 insertions(+), 4 deletions(-)

--
2.20.1


0001-isofs-truncate-oversized-Rock-Ridge-names-to-255-byt.patch

From 3d484405f0ad8d10ef490281da157bfdd7450cb6 Mon Sep 17 00:00:00 2001
From: Thomas Schmitt 
Date: Tue, 22 Sep 2020 12:34:50 +0200
Subject: [PATCH 1/1] isofs: truncate oversized Rock Ridge names to 255 bytes

Enlarge the limit for name bytes from 253 to 255.
Do not discard all bytes of the NM field where the overflow occurs, but
rather append them to the accumulated name before truncating it to exactly
255 bytes.
Map trailing incomplete UTF-8 bytes to '_'.

Signed-off-by: Thomas Schmitt 
---
 fs/isofs/rock.c | 

Re: Kernel bug tracker

[Thomas Schmitt]

Hi,

Adverg Ebashinskii wrote:
> If you could share your patch here to understand the problem better I would
> gladly dig into it.

Ok. Here is the simple one. The other comes in a separate mail.

The following texts stem from git format-patch. If submitting for real,
i would send them by git send-email to linux-ker...@vger.kernel.org and
linux-s...@vger.kernel.org.
(The latter because Jens Axboe committed a few isofs changes in the past
and because isofs is historically related to sr and cdrom.)


-cover-letter.patch

>From 154a68527351db091e5de60388ba4cfb1fe779fd Mon Sep 17 00:00:00 2001
From: Thomas Schmitt 
Date: Mon, 21 Sep 2020 18:20:14 +0200
Subject: [PATCH 0/1] isofs: prevent file time rollover after year 2038

The time values in struct inode of isofs result from calls to function
iso_date() in isofs/util.c, which returns seconds in the range of signed
int. This will rollover in 2038.
ISO 9660 directory record timestamps are good for up to year 2155.
(ECMA-119 9.1.5: 1900 + 255)

The only callers of iso_date() are in isofs/inode.c and isofs/rock.c
and put the result into struct inode.i_{a,c,m}time.tv_sec which is
of type time64_t.
The time value of iso_date() essentially stems from mktime64().

So return type time64_t is appropriate for iso_date().

--
Demonstration of the problem:

Create an ISO 9660 filesystem with file date in 2040, using file /bin/true
as victim payload:

  xorriso -outdev /tmp/test_date.iso \
  -blank as_needed \
  -map /bin/true /victim \
  -alter_date m 'Oct 01 22:06:12 2040' /victim --

Inspect the current representation by isofs:

  mount /tmp/test_date.iso /mnt/iso
  ls -l /mnt/iso/victim

This yields with int iso_date():

  ... Aug 26  1904 /mnt/iso/victim

After changing the type of iso_date() to time64_t:

  ... Oct  1  2040 /mnt/iso/victim

For completeness i tested the last possible second:

  xorriso ... -alter_date m 'Dec 31 23:59:59 2155' /victim --

and got properly:

  ... Dec 31  2155 /mnt/iso/victim

(When reproducing this it might be to wise to use December 30, to avoid
any potential timezone problems.)

--

Have a nice day :)

Thomas

Thomas Schmitt (1):
  isofs: prevent file time rollover after year 2038

 fs/isofs/isofs.h | 3 ++-
 fs/isofs/util.c  | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

--
2.20.1


0001-isofs-prevent-file-time-rollover-after-year-2038.patch

>From 154a68527351db091e5de60388ba4cfb1fe779fd Mon Sep 17 00:00:00 2001
From: Thomas Schmitt 
Date: Mon, 21 Sep 2020 18:20:06 +0200
Subject: [PATCH 1/1] isofs: prevent file time rollover after year 2038

Change the return type of function iso_date() from int to time64_t.

Signed-off-by: Thomas Schmitt 
---
 fs/isofs/isofs.h | 3 ++-
 fs/isofs/util.c  | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h
index 055ec6c586f7..527c0db72ff9 100644
--- a/fs/isofs/isofs.h
+++ b/fs/isofs/isofs.h
@@ -107,7 +107,8 @@ static inline unsigned int isonum_733(u8 *p)
/* Ignore bigendian datum due to broken mastering programs */
return get_unaligned_le32(p);
 }
-extern int iso_date(u8 *, int);
+
+time64_t iso_date(u8 *, int);

 struct inode;  /* To make gcc happy */

diff --git a/fs/isofs/util.c b/fs/isofs/util.c
index e88dba721661..348af786a8a4 100644
--- a/fs/isofs/util.c
+++ b/fs/isofs/util.c
@@ -16,10 +16,10 @@
  * to GMT.  Thus  we should always be correct.
  */

-int iso_date(u8 *p, int flag)
+time64_t iso_date(u8 *p, int flag)
 {
int year, month, day, hour, minute, second, tz;
-   int crtime;
+   time64_t crtime;

year = p[0];
month = p[1];
--
2.20.1



Have a nice day :)

Thomas


___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies