[VROC] [Ub 24.04] mdadm: buffer overflow detected

[Kinga Tanska]

[ Impact ]

mdadm crashes sporadically with error *** buffer overflow detected *** at some invokations:

- mdadm --detail-pl
- mdadm -CR /dev/md0 -l1 -n2 /dev/nvme0n1 /dev/nvme1n1
*** buffer overflow detected ***: terminated
Aborted (core dumped)

[ Test Plan ]

- Install mdadm
- Issue this command several times:
mdadm --detail-pl

[ Where problems could occur ]

The fix is very small and basically it replaces the unsafe functions call
to sprintf by calling snprintf for Intel platforms (platform_intel.c)
I do not expect high regression risk.

[ Other Info ]

mdadm is built with FORTIFY_SOURCE=3 (as it is done in Ubuntu 24.04).
and it uses the unsafe function sprintf() that will cause the
buffer-overflow error

It is fixed in mdadm upstream:
https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=827e1870f320545796d907f50af594e901399417

[Kinga Tanska (ktanska)]

Hello,
do you have any update?

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Mateusz Kusiak (mkusiak)]

Any updates on this?

[Hector CAO (hectorcao)]

Hello @ktanska and affected users,

A test package with the fix is available in this PPA:

https://launchpad.net/~hectorcao/+archive/ubuntu/2069821/

Could you help on confirming the fix ?

Thanks,

[Kinga Tanska (ktanska)]

Hi @hectorcao

I confirm that test package fixes this issue. We are looking forward official update

Thanks,
Kinga

[Timo Aaltonen (tjaalton)]

the test plan is missing the command to run

[Kinga Tanska (ktanska)]

Hi,

can you explain more what is needed? Do I have to prepare something?

This is the first issue resolved in mdadm from me, I don't know if I described everything correctly.

Thanks
Kinga

[Kinga Tanska (ktanska)]

Hi

I've updated test plan. Please notice, that this issue is reproducible only on Intel platforms

[Launchpad Janitor (janitor)]

This bug was fixed in the package mdadm - 4.3+20240412-1ubuntu2

---------------
mdadm (4.3+20240412-1ubuntu2) oracular; urgency=medium

  * d/p/lp2070371/: mdadm: wait for mdmon when it is stared via systemd (LP: #2070371)
  * d/p/lp2069821/: mdadm: buffer overflow detected (LP: #2069821)

 -- Hector Cao <email address hidden> Mon, 29 Jul 2024 10:06:31 +0200

[Mauricio Faria de Oliveira (mfo)]

Hi Hector,

Thanks for the work on this!

Despite these changes having already been in Oracular,
_please_ address these points for Noble/LTS for better
maintenance experience over a longer/LTS life-cycle.

Some details included; hope this helps in future uploads!

Thanks!

0) bug 2070371 has an incomplete SRU template (bug 2069821 is fine).

1) List the patch filenames in the changelog.
2) Use multiple patch files as sub-items.
3) Rename/use just filenames, not sub-dirs, for bug numbers.

e.g.,

   * mdadm: wait for mdmon when it is started via systemd (LP: #2070371)
     - d/p/lp2070371-0001-util.c-change-devnm-to-const-in-mdmon-functions.patch
     - d/p/lp2070371-0002-Wait-for-mdmon-when-it-is-stared-via-systemd.patch

   * mdadm: buffer overflow detected (LP: #2069821)
     - d/p/lp2069821-0001-mdadm-platform-intel-buffer-overflow-detected.patch

4) Add DEP-3 headers (please read [1]) to the patches; e.g.,
Bug-Ubuntu: https://bugs.launchpad.net/bugs/2069821
Origin: [upstream/backport,] https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=827e1870f320545796d907f50af594e901399417

5) Don't add empty lines to `d/p/series` (`quilt import` shouldn't)
6) Increment version by `ubuntu0.1` (not `ubuntu1`) in stable releases, in general
(Please read [2] for practical examples and use it as reference material in future).

[1] https://dep-team.pages.debian.net/deps/dep3/
[2] https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

[Bun K Tan (bktan1)]

@hectorcao - is this applied to Noble too?

[Hector CAO (hectorcao)]

@bktan1 : this feedback is mostly applied to Noble

[Simon Chopin (schopin)]

Hi Mauricio,

While some of your remarks are expected for an SRU review (SRU template, DEP-3, versioning, arguably changelog formatting), I'm bumping hard on the points 3 and 5.
AFAIK those are a matter of preference and tooling, and I have never come across any policy documentation mentioning either of those items.

It's fairly common to edit big series files by hand, in which case empty lines really help to structure that file. This is the first time I've seen someone complain about them!

If a package's patch set is maintained using gbp pq, it's natural to use subdirectories rather than plain file prefixes. I myself don't use that tool too often but still favour subdirs because, well, I find them more manageable long-term.

If this is documented, consensual policy then I'm reluctantly OK with changing my ways, but otherwise, could you please distinguish those cosmetic preferences from actual hard requirements? Otherwise it makes it unnecessarily hard for people that are still learning packaging.