Skip to content
Snippets Groups Projects
Commit 548f2c4d authored by Piotr Balcer's avatar Piotr Balcer
Browse files

obj: fix potential NULL-dereference in ulog_store

In situations where the total number of entries to be stored in the
ulog was an exact fit for the total capacity of the log,
the algorithm was unnecessarily trying to zero-out a non-existent
subsequent ulog. This triggered an ASSERT on debug builds and crashed
on NULL-dereferencing.
parent ec3960ae
No related branches found
No related tags found
No related merge requests found
/* /*
* Copyright 2016-2019, Intel Corporation * Copyright 2016-2020, Intel Corporation
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
...@@ -430,6 +430,7 @@ operation_process_persistent_redo(struct operation_context *ctx) ...@@ -430,6 +430,7 @@ operation_process_persistent_redo(struct operation_context *ctx)
ulog_store(ctx->ulog, ctx->pshadow_ops.ulog, ulog_store(ctx->ulog, ctx->pshadow_ops.ulog,
ctx->pshadow_ops.offset, ctx->ulog_base_nbytes, ctx->pshadow_ops.offset, ctx->ulog_base_nbytes,
ctx->ulog_capacity,
&ctx->next, ctx->p_ops); &ctx->next, ctx->p_ops);
ulog_process(ctx->pshadow_ops.ulog, OBJ_OFF_IS_VALID_FROM_CTX, ulog_process(ctx->pshadow_ops.ulog, OBJ_OFF_IS_VALID_FROM_CTX,
......
/* /*
* Copyright 2015-2019, Intel Corporation * Copyright 2015-2020, Intel Corporation
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
...@@ -292,8 +292,8 @@ ulog_checksum(struct ulog *ulog, size_t ulog_base_bytes, int insert) ...@@ -292,8 +292,8 @@ ulog_checksum(struct ulog *ulog, size_t ulog_base_bytes, int insert)
*/ */
void void
ulog_store(struct ulog *dest, struct ulog *src, size_t nbytes, ulog_store(struct ulog *dest, struct ulog *src, size_t nbytes,
size_t ulog_base_nbytes, struct ulog_next *next, size_t ulog_base_nbytes, size_t ulog_total_capacity,
const struct pmem_ops *p_ops) struct ulog_next *next, const struct pmem_ops *p_ops)
{ {
/* /*
* First, store all entries over the base capacity of the ulog in * First, store all entries over the base capacity of the ulog in
...@@ -312,9 +312,16 @@ ulog_store(struct ulog *dest, struct ulog *src, size_t nbytes, ...@@ -312,9 +312,16 @@ ulog_store(struct ulog *dest, struct ulog *src, size_t nbytes,
* If the nbytes is aligned, an entire cacheline needs to be addtionally * If the nbytes is aligned, an entire cacheline needs to be addtionally
* zeroed. * zeroed.
* But the checksum must be calculated based solely on actual data. * But the checksum must be calculated based solely on actual data.
* If the ulog total capacity is equal to the size of the
* ulog being stored (nbytes == ulog_total_capacity), then there's
* nothing to invalidate because the entire log data will
* be overwritten.
*/ */
size_t checksum_nbytes = MIN(ulog_base_nbytes, nbytes); size_t checksum_nbytes = MIN(ulog_base_nbytes, nbytes);
nbytes = CACHELINE_ALIGN(nbytes + sizeof(struct ulog_entry_base)); if (nbytes != ulog_total_capacity)
nbytes = CACHELINE_ALIGN(nbytes +
sizeof(struct ulog_entry_base));
ASSERT(nbytes <= ulog_total_capacity);
size_t base_nbytes = MIN(ulog_base_nbytes, nbytes); size_t base_nbytes = MIN(ulog_base_nbytes, nbytes);
size_t next_nbytes = nbytes - base_nbytes; size_t next_nbytes = nbytes - base_nbytes;
......
/* /*
* Copyright 2015-2018, Intel Corporation * Copyright 2015-2020, Intel Corporation
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
...@@ -126,6 +126,7 @@ int ulog_reserve(struct ulog *ulog, ...@@ -126,6 +126,7 @@ int ulog_reserve(struct ulog *ulog,
void ulog_store(struct ulog *dest, void ulog_store(struct ulog *dest,
struct ulog *src, size_t nbytes, size_t ulog_base_nbytes, struct ulog *src, size_t nbytes, size_t ulog_base_nbytes,
size_t ulog_total_capacity,
struct ulog_next *next, const struct pmem_ops *p_ops); struct ulog_next *next, const struct pmem_ops *p_ops);
void ulog_clobber(struct ulog *dest, struct ulog_next *next, void ulog_clobber(struct ulog *dest, struct ulog_next *next,
......
/* /*
* Copyright 2015-2018, Intel Corporation * Copyright 2015-2020, Intel Corporation
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
...@@ -353,12 +353,14 @@ FUNC_MOCK_END ...@@ -353,12 +353,14 @@ FUNC_MOCK_END
FUNC_MOCK(ulog_store, void, FUNC_MOCK(ulog_store, void,
struct ulog *dest, struct ulog *dest,
struct ulog *src, size_t nbytes, size_t redo_base_nbytes, struct ulog *src, size_t nbytes, size_t redo_base_nbytes,
size_t ulog_base_capacity,
struct ulog_next *next, const struct pmem_ops *p_ops) struct ulog_next *next, const struct pmem_ops *p_ops)
FUNC_MOCK_RUN_DEFAULT { FUNC_MOCK_RUN_DEFAULT {
switch (Ulog_fail) { switch (Ulog_fail) {
case FAIL_AFTER_FINISH: case FAIL_AFTER_FINISH:
_FUNC_REAL(ulog_store)(dest, src, _FUNC_REAL(ulog_store)(dest, src,
nbytes, redo_base_nbytes, nbytes, redo_base_nbytes,
ulog_base_capacity,
next, p_ops); next, p_ops);
DONEW(NULL); DONEW(NULL);
break; break;
...@@ -368,6 +370,7 @@ FUNC_MOCK(ulog_store, void, ...@@ -368,6 +370,7 @@ FUNC_MOCK(ulog_store, void,
default: default:
_FUNC_REAL(ulog_store)(dest, src, _FUNC_REAL(ulog_store)(dest, src,
nbytes, redo_base_nbytes, nbytes, redo_base_nbytes,
ulog_base_capacity,
next, p_ops); next, p_ops);
break; break;
} }
......
/* /*
* Copyright 2019, Intel Corporation * Copyright 2018-2020, Intel Corporation
* *
* Redistribution and use in source and binary forms, with or without * Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions * modification, are permitted provided that the following conditions
...@@ -179,6 +179,15 @@ test_redo(PMEMobjpool *pop, struct test_object *object) ...@@ -179,6 +179,15 @@ test_redo(PMEMobjpool *pop, struct test_object *object)
(struct ulog *)&object->redo, TEST_ENTRIES, (struct ulog *)&object->redo, TEST_ENTRIES,
pmalloc_redo_extend, NULL, &pop->p_ops, LOG_TYPE_REDO); pmalloc_redo_extend, NULL, &pop->p_ops, LOG_TYPE_REDO);
/*
* Keep this test first.
* It tests a situation where the number of objects being added
* is equal to the capacity of the log.
*/
test_set_entries(pop, ctx, object, TEST_ENTRIES - 1,
FAIL_NONE, LOG_PERSISTENT);
clear_test_values(object);
test_set_entries(pop, ctx, object, 10, FAIL_NONE, LOG_PERSISTENT); test_set_entries(pop, ctx, object, 10, FAIL_NONE, LOG_PERSISTENT);
clear_test_values(object); clear_test_values(object);
test_merge_op(ctx, object); test_merge_op(ctx, object);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment