Skip to content

Test #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fsuba wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Test #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a55d9f
Test
fsuba May 28, 2025
20bf3d1
test2
fsuba May 28, 2025
f634dbd
Test 3
fsuba May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/c++/vdo/base/logical-zone.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ void vdo_free_logical_zones(struct logical_zones *zones)
return;

vdo_free(vdo_forget(zones->manager));

vdo_free(zones);
Comment on lines +136 to +137
Copy link

@coderabbitai coderabbitai bot May 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical use-after-free bug introduced by reordering memory deallocation.

Moving vdo_free(zones) before the loop creates a use-after-free vulnerability. After zones is freed on line 137, the subsequent loop on lines 139-140 accesses zones->zone_count and zones->zones[index], which is undefined behavior and can lead to crashes or security issues.

Apply this diff to fix the use-after-free bug:

 	vdo_free(vdo_forget(zones->manager));
-	
-	vdo_free(zones);
 
 	for (index = 0; index < zones->zone_count; index++)
 		vdo_int_map_free(vdo_forget(zones->zones[index].lbn_operations));
+
+	vdo_free(zones);

The zones structure must remain valid until after all its members (zone_count and zones[index].lbn_operations) have been accessed.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vdo_free(zones);
vdo_free(vdo_forget(zones->manager));
for (index = 0; index < zones->zone_count; index++)
vdo_int_map_free(vdo_forget(zones->zones[index].lbn_operations));
vdo_free(zones);
🤖 Prompt for AI Agents 
In src/c++/vdo/base/logical-zone.c around lines 136 to 137, the call to
vdo_free(zones) is placed before a loop that accesses members of the zones
structure, causing a use-after-free bug. To fix this, move the vdo_free(zones)
call to after the loop that uses zones->zone_count and
zones->zones[index].lbn_operations, ensuring the zones structure remains valid
while it is accessed.


for (index = 0; index < zones->zone_count; index++)
vdo_int_map_free(vdo_forget(zones->zones[index].lbn_operations));

vdo_free(zones);
}

static inline void assert_on_zone_thread(struct logical_zone *zone, const char *what)
Expand Down
4 changes: 1 addition & 3 deletions src/c++/vdo/tests/BlockMapRecovery_t1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ enum {
};

static size_t entryCount = 0;
static struct repair_completion *repair_completion = NULL;

/**
* Initialize the index, vdo, and test data.
Expand Down Expand Up @@ -75,7 +74,7 @@ static bool preventReferenceCountRebuild(struct vdo_completion *completion)
struct vdo_completion *parent = completion->parent;
int result = completion->result;

repair_completion = (struct repair_completion *) completion;
free_repair_completion((struct repair_completion *) completion);
vdo_fail_completion(parent, result);
return false;
}
Expand Down Expand Up @@ -156,7 +155,6 @@ static void testRecovery(size_t desiredEntryCount)
// Do a block map recovery.
setCompletionEnqueueHook(hijackJournalLoad);
performSuccessfulAction(vdo_repair);
free_repair_completion(repair_completion);

// Verify that all block map mappings are either the original value or the
// new mapping expected from recovery.
Expand Down
2 changes: 1 addition & 1 deletion src/c++/vdo/user/vdoStats.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ static void displayDFStyle(const char *path, struct vdo_statistics *stats)

// Extract the device name. Use strdup for non const string.
char *devicePath = strdup(path);
strcpy(dfName, basename(devicePath));
free(devicePath);
strcpy(dfName, basename(devicePath));
Comment on lines 183 to +184
Copy link

@coderabbitai coderabbitai bot May 29, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Fix use-after-free vulnerability.

The code is calling free(devicePath) on line 183 and then immediately using devicePath in basename(devicePath) on line 184. This is undefined behavior that can lead to crashes or security vulnerabilities.

Apply this diff to fix the use-after-free issue:

-  free(devicePath);
-  strcpy(dfName, basename(devicePath));
+  strcpy(dfName, basename(devicePath));
+  free(devicePath);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
free(devicePath);
strcpy(dfName, basename(devicePath));
strcpy(dfName, basename(devicePath));
free(devicePath);
🤖 Prompt for AI Agents 
In src/c++/vdo/user/vdoStats.c around lines 183 to 184, the code frees
devicePath and then uses it in basename(devicePath), causing a use-after-free
vulnerability. To fix this, call basename(devicePath) and copy its result into
dfName before freeing devicePath. This ensures devicePath is valid when passed
to basename and prevents undefined behavior.


// Display the device statistics
if (!headerPrinted) {
Expand Down