Skip to content

Fix memory leaks in create_resource_pool functions - #63

Open
Mrcopytuo wants to merge 10000 commits into
NetBSD:trunkfrom
Mrcopytuo:fix-resouce-pool
Open

Fix memory leaks in create_resource_pool functions#63
Mrcopytuo wants to merge 10000 commits into
NetBSD:trunkfrom
Mrcopytuo:fix-resouce-pool

Conversation

@Mrcopytuo

Copy link
Copy Markdown

Fix missing kfree(pool) in resource pool creation paths (CVE-2019-19082)

Several resource pool creation functions were missing a kfree(pool) call when their corresponding *_construct() routines failed. In some cases, the free was placed after BREAK_TO_DEBUGGER(). This results in memory leaks on construction failure.

These issues mirror the leak pattern described in CVE-2019-19082. This patch ensures the pool is always freed properly before returning NULL.

Changes made:

  • dcn20_resource.c:

    • Move kfree(pool) before BREAK_TO_DEBUGGER().
  • dcn21_resource.c:

    • Move kfree(pool) before BREAK_TO_DEBUGGER().
  • dce80_resource.c:

    • Add missing kfree(pool) before BREAK_TO_DEBUGGER() in:
      • dce80_create_resource_pool()
      • dce81_create_resource_pool()
      • dce83_create_resource_pool()

thorpej and others added 30 commits November 24, 2025 16:36
…also

use machine_bootmap[]", just remove the option (it looks like it would
be opted into on every platform).
This includes the equivalent of the local changes that were
introduced in r1.14 of elfdefinitions.m4.
Apparently gcc doesn't do this automatically, and it's noticably
faster on sparc64 (several seconds less to encrypt 128mb of data).
the handles for I2C and SPI were split out into attachment specific
softc structures.  This was all good and fine and a nice improvement.
However, the CFATTACH_DECL_ call still included the general softc in
the I2C case and not the I2C attachment specific softc.  This would
probably overwrite some of the config information in the kernel, but
might not show itself to be a problem unless you did a detach /
reattach operation.  Fix this up.
when you have a bus that can detach, such as presented by the
umcpmio(4) driver.

o The dev/ic/com.c code pretty much assumed that it was using a device
on a computer bus and as such the registers would not suddenly
disappear out from under it.  This won't be the case with the
sc16is7xx(4) driver.  However, the code in dev/ic/com.c is not at all
structured in a manor that handles a failure to do a read or write.
Generally, this won't be a problem, except for comintr() which is the
interrupt handler.  When called in polled mode, especially, it is
possible that the code in comintr() can get stuck in one of its loops
because it will NEVER get the values it expects back from the device
if the device disappears.  To deal with this introduce a out of band
way to indicate that a read or write failed and check that in
comintr().  This allows it to exit out in a sane manor.  This along
with the corresponding changes to the attachment specific code fixes a
problem where the system could (and probably would) hang up hard if
the device disappeared (comintr() would be running in a VERY tight
loop hogging everything).  Note that the enable / disable support that
com.c has wasn't enough to deal with this, as the call into comintr()
could already have been in flight and well on its way to getting
stuck.


o Use the proper softc in the CFATTACH_DECL_ call for the I2C and SPI
attachments.  I don't know what I was thinking at the time.  Fixes a
problem where the system will panic in a unusual way if the device is
detached and then reattached.
The esym value in locore was changed to be loaded into an address
register (%a4), but on m68k the MOVEA instruction does not update the
condition codes. The subsequent `jne LstartX` therefore relied on stale
CCR flags rather than testing whether esym was zero.

Add an explicit `tstl %a4` so that the Z flag is updated from the esym
value before branching, ensuring the correct end-of-kernel address and
nextpa are computed.
This makes reboot work on HP9000/362 with the new pmap.
It seems these lines should have been removed (or not been added later)
when Hibler's hp300 updates (MAXADDR was changed always mapped,
and 68040 support) were committed to 4.4BSD in 1990 and 1992:
 https://svnweb.freebsd.org/csrg?view=revision&revision=45751
 https://svnweb.freebsd.org/csrg/sys/hp300/hp300/locore.s?r1=45359&r2=45751
 https://svnweb.freebsd.org/csrg?view=revision&revision=53933
 https://svnweb.freebsd.org/csrg/sys/hp300/hp300/locore.s?r1=50544&r2=53933

Probably harmless (PROM doesn't refer the bogus written value),
and no visible hehavior change on HP9000/382 during reboot.
…_NC.

Kernel source code does not use this constant, so this change should
not affect its runtime behavior.  Userland toolchains use the
ABI-defined value for this relocation, via their own definitions.

Remove a duplicate definition for R_AARCH64_TLSLD_ADD_DTPREL_HI12.
(They're not 100% identical across all m68k ports.)
Figure 9-8. Address Translation General Flowchart, ATC hit, B==1 case,
as well as 68040UM Figure 3-21. ATC Entry and Tag Fields, R bit and the
associated descriptive text).

As such, we need to make sure to invalidate the ATC entry even for
new mappings where previously one did not exist, because, in the
case of a demand-page-in, there will almost certainly be a negative
ATC entry lurking about.

Many thanks to isaki@ for his analysis of the issue.
…case,

but there's basically zero documentation for it available, so we err on the
side of caution.
* pmap_pv_enter(), when page is already marked as CI: This one is required
  for the same reason as the previous change was required.

* pmap_enter(): Also need an ATC hit in the case of a non-managed physical
  address (demand-page-in of device mapping?)

* pmap_kenter_pa(): Add an ATC hit here after storing the PTE, but this is
  really just over-caution, I think; we're not dealing with VA regsions
  that would have been subject to a demand-page-in, so there should not
  be any negative entries to worry about.  Possible future optimization.

* pmap_zero_page(), pmap_copy_page(): Same situation as pmap_kenter_pa().
  These addresses are even more tightly controlled, so this is probably
  the first "probably uncessary ATC hit" that should be removed once this
  implementation is shown to be stable across a variety of machines.

Add a comment in pmap_enter() for the case where an old mapping must be
removed before inserting the new one: since we're going to hit the ATC
after storing the new PTE, we can probably elide the ATC hit when removing
this mapping.  Possible future optimization.
see if indirect match and attach works.

However, it had an aggressive panic in its piixpm_i2c_exec() function
if there existed a command but not any data.  Turn this into a
return(EINVAL) sort of thing.  Without this, just modloading aht20temp
will panic a NetBSD/amd64 Virtualbox VM.

It is very common for I2C devices, especially sensor devices, to do a
READ (with or without STOP) with just a command sent, wait a while and
then do a WRITE or more READ without a command and with a data buffer.
Often the device has to have this pause in order to perform some
action.

I don't have the datasheet on the piixpm(4) chip available to me, and
it is possible that it can handle a command without data.  The
interrupt function implies as much, but for now, just EINVAL the
situation.

It would have also been trivial to panic the system from userland by
opening the iic device and performing ioctl calls and it would be best
to not allow that.
gcc on most netbsd architectures (mostly excluding sparc and vax) can
generate fast MD byte-swapping code. On sparc it generates a function
call, which is very slow. We have existing inline macros for byte
swapping, so use those instead.

bswap on sparc with gcc is still non-ideal, so also reactivate the
portable endian encoding functions we apparently haven't been using
since the gcc2 days, rather than using bswap for that.

These changes improve the speed of disk encryption on ultrasparc ii
by almost 40% (the endian.h change being more significant in making
that number go high).

clang does the right thing on sparc (or at least introduces different
bottlenecks...), so avoid this there.

Thanks joerg for analysis of the problem, uwe for reviewing the patch.

PR port-sparc64/59789 bswap is slow
PR kern/59774 bearssl 32-bit AES is too slow
perseant and others added 26 commits December 10, 2025 03:08
rather than trying to coordinate two separate events.
While here, document these local definitions.

PR lib/59564
instead of checking the SD_IO_RW_EXTENDED command op code.

Also add this logic to sunxi_mmc.
…number 1

into regular files.  The Ifile has not been exposed in the file system for
more than 15 years, and this conversion makes it impossible to have whiteouts
at the root level.
…allback;

and explicitly check lfs_iocount before destroying the pool at unmount.
Prevents an occasional "pool busy" panic at unmount time.
tested on rockpro64 and pinebookpro.
(Forgot to commit this file with rev. 1.148 of locore.s)
When parsing resolv.conf entries we build up shell variables.
Because this is done via a pipe, we need to echo the variables
to stdout and eval the result to get them into the main resolvconf.
We have no idea what the values are, so we build up the output
ensuring the parsed value is single quoted so eval will always
interpret it as a string and nothing more.

This avoids an attack like so:
        `echo 'search $(touch /tmp/foo)' | resolvconf -a bar`
* libc: add toggle resolv_conf_restore, defaulting to YES
* resolvconf: Fix -D
* resolvconf: Don't warn when we have no entries to list for *
* resolvconf: -I now inits subscribers after clearing state
* resolvconf: remember if any subscriber errored
* libc: Don't update resolv.conf on signature mismatch
* resolvconf: Single quote parsed values from resolv.conf
Make it possible to search for literal two colons (::) and actually
find something.  Make the "x"/"x:"/"x::" examples more explicit and
more visibile.

PR lib/59828
With this change:

- Elftoolchain's <sys/elfdefinitions.h> is used for the definitions
  that make up the ELF gABI and its processor-specific ABI
  supplements.

- Existing NetBSD-local definitions are brought in via
  the kernel header <sys/exec_elf.h>, for continuity.

This changeset prepares the ground for migrating userland-only
definitions out of the kernel header, for better separation
of concerns.

This changeset also eases the development of cross-architecture
ELF tools since <elf.h> now offers ELF-related constants for all
of the architectures that NetBSD currently supports.

PR lib/59564
ricardobranco777 pushed a commit to ricardobranco777/netbsd-src that referenced this pull request Feb 14, 2026
…D#63):

	sys/dev/pci/pvscsi.c: revision 1.3

pvscsi(4): Use paravirt_membar_sync(9) where needed.

PR kern/59618: occasional virtio block device lock ups/hangs
(pvscsi(4) is not virtio(4) but the same fundamental issue arises.)
netbsd-srcmastr pushed a commit that referenced this pull request Jul 26, 2026
	sys/dev/pci/pvscsi.c: revision 1.3

pvscsi(4): Use paravirt_membar_sync(9) where needed.

PR kern/59618: occasional virtio block device lock ups/hangs
(pvscsi(4) is not virtio(4) but the same fundamental issue arises.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.