Skip to content

Fix possible use-after-free on reference count and some minor CI changes #130

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
leonardoheld wants to merge 3 commits into
base: master
Choose a base branch
from
+17 −6
Open

Fix possible use-after-free on reference count and some minor CI changes #130

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
0152acd
Prevent workflow that updates containers from running on forks
leonardoheld Feb 17, 2025
736326f
Use the short hash instead of version if tags are not available durin…
leonardoheld Feb 17, 2025
bad4660
Fix possible use-after-free on reference count
leonardoheld Feb 14, 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: 3 additions & 1 deletion .github/workflows/update-docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
name: Aktualizr CI docker images update
on:
push:
branches: [ master ]
branches: [master]
jobs:
update-bionic:
name: Update Ubuntu Bionic Image
if: github.repository == 'uptane/aktualizr'
runs-on: ubuntu-latest
env:
DOCKER_TAG: docker.pkg.github.com/uptane/aktualizr/aktualizr-ci:bionic-master
Expand All @@ -21,6 +22,7 @@ jobs:

update-ubuntu-focal:
name: Update Ubuntu Focal Image
if: github.repository == 'uptane/aktualizr'
runs-on: ubuntu-latest
env:
DOCKER_TAG: docker.pkg.github.com/uptane/aktualizr/aktualizr-ci:ubuntu-focal-master
Expand Down
6 changes: 5 additions & 1 deletion scripts/get_version.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ set -euo pipefail
GIT=${1:-git}
REPO=${2:-.}

"$GIT" -C "$REPO" describe --long | tr -d '\n'
if ! VERSION=$("$GIT" -C "$REPO" describe --long 2>/dev/null | tr -d '\n'); then
VERSION=$("$GIT" -C "$REPO" rev-parse --short HEAD | tr -d '\n')
fi

printf "%s" "$VERSION"
13 changes: 9 additions & 4 deletions src/libaktualizr-posix/asn1/asn1_message.h
View file
Open in desktop
Copy link
Copy Markdown
Collaborator

@rborn-tx rborn-tx Apr 2, 2025

Choose a reason for hiding this comment

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

@leonardoheld I imagine you'll drop the changes to this file since the actual issue in completely unrelated to multi-threading, right?

Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#ifndef ASN1_MESSAGE_H_
#define ASN1_MESSAGE_H_

#include <atomic>

#include <boost/intrusive_ptr.hpp>

#include "AKIpUptaneMes.h"
Expand Down Expand Up @@ -55,9 +58,12 @@ class Asn1Message {
*/
static Asn1Message::Ptr FromRaw(AKIpUptaneMes_t** msg) { return new Asn1Message(msg); }

friend void intrusive_ptr_add_ref(Asn1Message* m) { m->ref_count_++; }
friend void intrusive_ptr_add_ref(Asn1Message* m) { m->ref_count_.fetch_add(1, std::memory_order_relaxed); }

friend void intrusive_ptr_release(Asn1Message* m) {
if (--m->ref_count_ == 0) {
const int prev_count = m->ref_count_.fetch_sub(1, std::memory_order_release);
if (prev_count == 1) {
std::atomic_thread_fence(std::memory_order_acquire);
delete m;
}
}
Expand Down Expand Up @@ -144,8 +150,7 @@ class Asn1Message {
AKIpUptaneMes_t msg_{}; // Note that this must be zero-initialized

private:
int ref_count_{0};

std::atomic<int> ref_count_{0};
Asn1Message() = default;

explicit Asn1Message(AKIpUptaneMes_t** msg) {
Expand Down