Project

General

Profile

Bug #8554

Small ipsec_alginfo_t leak when zones shutdown

Added by Dan McDonald over 3 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
networking
Start date:
2017-08-02
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

Consider a zone with IPsec loaded.

It maintains an array of algorithms, based on two 256-element arrays (authentication and encryption). Here's an example:

>  0xffffff025b3d2000::print -L ipsec_stack_t ipsec_alglists
ipsec_alglists = [
    [ 0xffffff02692b6668, 0, 0xffffff02692b65e0, 0xffffff02692b6558, 0, 
0xffffff02692b64d0, 0xffffff02692b6448, 0xffffff02692b63c0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0 ]
    [ 0xffffff02692b6338, 0, 0xffffff02692b62b0, 0xffffff02692b6228, 0, 0, 0, 
0xffffff02692b61a0, 0, 0, 0, 0xffffff02692b6118, 0xffffff02692b6090, 0, 
0xffffff02692b6008, 0xffffff02695f6f78, 0xffffff02695f6ef0, 0, 
0xffffff02695f6e68, 0xffffff02695f6de0, 0xffffff02695f6d58, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]
]
> 

Now look at the ipsec_stack_fini() code that cleans things up:

    for (algtype = 0; algtype < IPSEC_NALGTYPES; algtype ++) {
        int nalgs = ipss->ipsec_nalgs[algtype];

        for (i = 0; i < nalgs; i++) {
            if (ipss->ipsec_alglists[algtype][i] != NULL)
                ipsec_alg_unreg(algtype, i, ns);
        }
    }

Instead of brute-forcing all of the entries, it only goes up to the number of available ones. This will leave dangling references because the algorithm tables have NULL-pointer gaps in them.

Brute-forcing the whole 512 elements is simpler, and since it's on a teardown path, not a candidate for being efficient.

#1

Updated by Electric Monk over 3 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 9b4fe9cb6a3fd5b562f036d9bf405f30c095808e

commit  9b4fe9cb6a3fd5b562f036d9bf405f30c095808e
Author: Dan McDonald <danmcd@joyent.com>
Date:   2017-08-02T23:40:44.000Z

    8554 Small ipsec_alginfo_t leak when zones shutdown
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF