sin/samba-mirror
1
0
Fork 0
mirror of https://github.com/samba-team/samba.git synced 2025-03-10 12:58:35 +03:00
Code

autorid: fix a potential for data corruption.

Browse Source 
The initialization of the HWM values in autorid.tdb was racy:

It did:

1. fetch the HWM value
2. if it did not exist, store 0 in a transaction.

This can be racy if two processes at the same time try to
run the initialization code, especially in a cluster, when
winbindd and smbd are started simultaneously on all nodes.
The race is that the HWM is not re-fetched inside the transaction.

Assume both processes see that the HWM does not exist.
Both try to start a transaction. Process 1 gets the lock
and process 2 blocks. After Process 1 has stored the
HWM, it proceeds and manages to start subsequent transactions
which also bump the HWM value (e.g. a range allocation,
which is also triggered from allocation code). When
process 2 finally manages to start the transaction, the
HWM value is aready > 0. But process 2 does not look again
and simply overwrites the HWM with 0.

So the next allocation will overwrite an existing mapping,
at least partially.

This patch changes the mechanism to:

1. fetch the hwm value
2. if it does not exist start a transaction
3.   fetch the hwm value
4.   if it does not exist, store 0
5. commit the transaction.

Note: this is not theoretical. Corruptions have been
seen in cluster environments.

Signed-off-by: Michael Adam <obnox@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
This commit is contained in:
Michael Adam 2014-03-20 12:07:19 +01:00 committed by Jeremy Allison
parent 758308c25a
commit 0bfb078719
1 changed files with 33 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
source3/winbindd/idmap_autorid_tdb.c
View File

@ -389,6 +389,37 @@ NTSTATUS idmap_autorid_get_domainrange(struct db_context *db,
}
/* initialize the given HWM to 0 if it does not exist yet */
static NTSTATUS idmap_autorid_init_hwm_action(struct db_context *db,
void *private_data)
{
NTSTATUS status;
uint32_t hwmval;
const char *hwm;
hwm = (char *)private_data;
status = dbwrap_fetch_uint32_bystring(db, hwm, &hwmval);
if (NT_STATUS_IS_OK(status)) {
DEBUG(1, ("HWM (%s) already initialized in autorid database "
"(value %"PRIu32").\n", hwm, hwmval));
return NT_STATUS_OK;
}
if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
DEBUG(0, ("Error fetching HWM (%s) from autorid "
"database: %s\n", hwm, nt_errstr(status)));
return status;
}
status = dbwrap_trans_store_uint32_bystring(db, hwm, 0);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("Error storing HWM (%s) in autorid database: %s\n",
hwm, nt_errstr(status)));
return status;
}
return NT_STATUS_OK;
}
NTSTATUS idmap_autorid_init_hwm(struct db_context *db, const char *hwm)
{
NTSTATUS status;
@ -406,7 +437,8 @@ NTSTATUS idmap_autorid_init_hwm(struct db_context *db, const char *hwm)
return status;
}
status = dbwrap_trans_store_uint32_bystring(db, hwm, 0);
status = dbwrap_trans_do(db, idmap_autorid_init_hwm_action,
(void *)hwm);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("Error initializing HWM (%s) in autorid database: "
"%s\n", hwm, nt_errstr(status)));