Skip to content

Enforce OAPV_MAX_NUM_META_PAYLOADS limit in oapvm_set() - #293

Merged
kpchoi merged 1 commit into
AcademySoftwareFoundation:mainfrom
fkyslov:fix-metadata-payload-count-limit
Sep 29, 2026
Merged

kpchoi merged 1 commit into
AcademySoftwareFoundation:mainfrom
fkyslov:fix-metadata-payload-count-limit

Conversation

@fkyslov

@fkyslov fkyslov commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Enforces the maximum metadata payload count in oapvm_set() (src/oapv_metadata.c):

  • Check md->mdp_num < OAPV_MAX_NUM_META_PAYLOADS and return OAPV_ERR_REACHED_MAX before allocating and appending a new metadata payload entry.

Testing

  • Verified all 24 ctest unit/conformance tests pass with AddressSanitizer and UndefinedBehaviorSanitizer.

Comment thread src/oapv_metadata.c Outdated

oapv_mdp_t *mdp_t = meta_find_mdp(md, type, uuid);
if(mdp_t == NULL) { // add new one
oapv_assert_gv(md->mdp_num < OAPV_MAX_NUM_META_PAYLOADS, ret, OAPV_ERR_REACHED_MAX, ERR);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OAPV_MAX_NUM_META_PAYLOADS is defined as the maximum number of metadata payloads per access unit, not per metadata group. oapvm_get_all() also checks it against the total over all groups. With a per-group check, up to 16 × 128 payloads can still be accepted by oapvm_set(), and the error only shows up later in oapvm_get_all().

It is suggested that the check count the payloads of all groups instead. Since oapvm_get_all() already has the same loop, a small static helper can be shared by both:

static int meta_get_num_mdp(oapvm_ctx_t *ctx)
{
    int num = 0;
    for(int i = 0; i < ctx->num; i++) {
        num += ctx->md_arr[i].mdp_num;
    }
    return num;
}

and in oapvm_set():

oapv_assert_gv(meta_get_num_mdp(ctx) < OAPV_MAX_NUM_META_PAYLOADS, ret, OAPV_ERR_REACHED_MAX, ERR);

…et()

Signed-off-by: Fyodor Kyslov <kyslov@google.com>
@fkyslov
fkyslov force-pushed the fix-metadata-payload-count-limit branch from d6f7ced to 015fd1a Compare September 28, 2026 17:58
@fkyslov

fkyslov commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Updated per review feedback: added meta_get_num_mdp(ctx) to count total metadata payloads across all groups in the access unit, enforced meta_get_num_mdp(ctx) < OAPV_MAX_NUM_META_PAYLOADS in oapvm_set(), and reused meta_get_num_mdp(ctx) in oapvm_get_all().

@kpchoi kpchoi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kpchoi
kpchoi merged commit 23a1791 into AcademySoftwareFoundation:main Sep 29, 2026
10 checks passed
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.

2 participants