Merge ~utkarsh/ubuntu/+source/libapache2-mod-perl2:fix-sigsev-perl-parse into ubuntu/+source/libapache2-mod-perl2:ubuntu/focal-devel

[Utkarsh Gupta]

Hello,

This MP intends to fix a SIGSEGV crash due to wrong use of perl_parse() (cf: LP: #1915959). This has already been fixed in hirsute and this MP is backporting the same upstream patch to Focal.

PPA at https://launchpad.net/~utkarsh/+archive/ubuntu/experimental-dump.

The autopkgtest is happy:
```
autopkgtest [19:34:05]: @@@@@@@@@@@@@@@@@@@@ summary
mysmoke PASS
autodep8-perl-build-deps SKIP exit status 77 and marked as skippable
autodep8-perl PASS (superficial)
autodep8-perl-recommends SKIP exit status 77 and marked as skippable
```

Should you need any more details, let me know. Requesting you to please review and sponsor the upload.

[Bryce Harrington (bryce)]

* Changelog:
  - [-] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [x] update-maintainer has been run

* Actual changes:
  - [-] no upstream changes to consider
  - [-] no further upstream version to consider
  - [ ] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [-] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [-] no new patches added
  - [√] patches match what was proposed upstream
  - [√] patches correctly included in debian/patches/series
  - [√] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [ ] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [-] sanity checks test fine

Verified build and autopkgtest work fine locally for me too.

Don't forget to run `update-maintainer`, which is required when you're making the first modification on a package sync'd from debian. (E.g. whenever you have a *ubuntu1 or *ubuntu0.1 or similar version).

Otherwise LGTM, +1. Will sponsor the upload directly.

review: Approve

[Bryce Harrington (bryce)]

   - [-] debian changes look safe
   - [√] verified PPA package installs/uninstalls

(Forgot to checkoff those items.)

[Bryce Harrington (bryce)]

Tagged, pushed, and upload sponsored:

stirling:~/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2-gu$ rm debian/files
stirling:~/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2-gu$ git ubuntu tag --upload
stirling:~/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2-gu$ git push pkg upload/2.0.11-2ubuntu0.20.04.1
cd ..
Enumerating objects: 21, done.
Counting objects: 100% (21/21), done.
Delta compression using up to 12 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 2.37 KiB | 2.37 MiB/s, done.
Total 15 (delta 10), reused 1 (delta 0)
ls *source.To ssh://git.launchpad.net/ubuntu/+source/libapache2-mod-perl2
 * [new tag] upload/2.0.11-2ubuntu0.20.04.1 -> upload/2.0.11-2ubuntu0.20.04.1
stirling:~/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2-gu$ cd ..
stirling:~/pkg/Libapache2ModPerl2/review-mp399921$ ls *source.changes
libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.changes
stirling:~/pkg/Libapache2ModPerl2/review-mp399921$ debsponsor libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.changes
The .changes file is already signed.
Would you like to use the current signature? [Yn]y
Leaving current signature unchanged.
stirling:~/pkg/Libapache2ModPerl2/review-mp399921$ dput ubuntu libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.changes
Checking signature on .changes
gpg: /home/bryce/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: /home/bryce/pkg/Libapache2ModPerl2/review-mp399921/libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1.dsc: done.
  Uploading libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1.debian.tar.xz: done.
  Uploading libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.buildinfo: done.
  Uploading libapache2-mod-perl2_2.0.11-2ubuntu0.20.04.1_source.changes: done.
Successfully uploaded packages.

[Bryce Harrington (bryce)]

### SRU paperwork review ###

Some tips based on my experience filing SRUs...

The Impact section should be kept super short and concise. It just needs to state the use case that sees the bug, and what the effect is (from a user POV). The first sentence in your Impact section is perfect, you probably don't need to say much more.

The discussion of the debugging work done is great, but should be moved down to a [Discussion] section at the end (or an [Original Report] section if preferred). The SRU review team is less interested in knowing how the bug got fixed, than they are with the other aspects, to from their POV that's all just background detail. For Impact they want to quickly understand "how bad is this bug, and is it worth my attention?"

The [Test Plan] section looks good. I usually direct them to install via 'apt', test, then use 'add-apt-repository -yus <ppa>', upgrade the package and re-test, but this works too.

One possible improvement might be to list using valgrind like in the original report, to show not just that it's crashing, but note exactly where in the code its crashing. I.e.:

    # valgrind apache2 -k start -X
    ...
    ==22529== Invalid read of size 8
    ==22529== at 0x564AFF5: perl_parse (in /usr/lib/x86_64-linux-gnu/libperl.so.5.30.0)
    ...

Reasoning is that there are some situations that crash a certain way, you fix it, and it still crashes but somewhere else or under certain conditions. So being specific can help more quickly isolate those situations when they crop up.

For the [Where problems could occur] section, for crashes like these I usually same some variation of: The patch fixes buffer termination issue and as such things to watch for in testing include the usual pointer handling problems like crashes during startup or runtime, or weird memory related issues.

[Utkarsh Gupta (utkarsh)]

Oh thanks, Bryce. These are some great pointers. Thanks for the help, I'll keep these in mind when filing my net SRU! \o/

Also, thanks for the sponsorship! :D

diff --git a/debian/changelog b/debian/changelog
index 4e549fa..d23fc41 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+libapache2-mod-perl2 (2.0.11-2ubuntu0.20.04.1) focal; urgency=medium
+
+  * Fix a SIGSEGV crash. (LP: #1915959)
+    - d/p/Fix_SIGSEGV_perl_parse.patch: Add a patch from upstream
+      SVN to fix a SIGSEGV crash due to wrong use of perl_parse().
+      + Thanks, Charles Pigott, for the patch.
+
+ -- Utkarsh Gupta <[e-mail]>  Fri, 19 Mar 2021 19:00:24 +0530
+
 libapache2-mod-perl2 (2.0.11-2) unstable; urgency=medium
 
   [ gregor herrmann ]
diff --git a/debian/patches/Fix_SIGSEGV_perl_parse.patch b/debian/patches/Fix_SIGSEGV_perl_parse.patch
new file mode 100644
index 0000000..ae1d274
--- /dev/null
+++ b/debian/patches/Fix_SIGSEGV_perl_parse.patch
@@ -0,0 +1,29 @@
+Description: Fix SIGSEGV crash due to wrong use of perl_parse()
+Origin: https://svn.apache.org/viewvc?view=revision&revision=1886793
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/libapache2-mod-perl2/+bug/1915959
+Author: Charles Pigott <[e-mail]>
+Reviewed-by: gregor herrmann <[e-mail]>
+Last-Update: 2021-02-22
+
+--- a/src/modules/perl/modperl_config.c
++++ b/src/modules/perl/modperl_config.c
+@@ -163,7 +163,8 @@
+     scfg->PerlPostConfigRequire =
+         apr_array_make(p, 1, sizeof(modperl_require_file_t *));
+ 
+-    scfg->argv = apr_array_make(p, 2, sizeof(char *));
++    /* 2 arguments + NULL terminator */
++    scfg->argv = apr_array_make(p, 3, sizeof(char *));
+ 
+     scfg->setvars = apr_table_make(p, 2);
+     scfg->configvars = apr_table_make(p, 2);
+@@ -219,6 +220,9 @@
+ 
+     *argc = scfg->argv->nelts;
+ 
++    /* perl_parse() expects a NULL terminated argv array */
++    modperl_config_srv_argv_push(NULL);
++
+     MP_TRACE_g_do(dump_argv(scfg));
+ 
+     return (char **)scfg->argv->elts;
diff --git a/debian/patches/series b/debian/patches/series
index a08118a..99ee5a5 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -12,3 +12,4 @@ avoid-db-linkage.patch
 360-conditional-linux-pid-module.patch
 0001-Skip-t-protocol-pseudo_http.t-incompatible-with-Apac.patch
 honour-env-LDFLAGS.patch
+Fix_SIGSEGV_perl_parse.patch