Skip to content

Commit 18941ec

Browse files
committed
cami: Check for and prevent resource leaks.
* Also test simpleami under valgrind. * Upgrade from actions/checkout@v4 to v6. * Fix duplicate fd closure in ami_cleanup.
1 parent 180bb3a commit 18941ec

2 files changed

Lines changed: 24 additions & 17 deletions

File tree

.github/workflows/main.yml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,16 @@ on:
1212

1313
# A workflow run is made up of one or more jobs that can run sequentially or in parallel
1414
jobs:
15-
debian-12:
15+
debian-13:
1616
runs-on: ubuntu-latest
17-
name: Debian 12
18-
container: debian:12
17+
name: Debian 13
18+
container: debian:13
1919
steps:
2020
- name: Build Asterisk
2121
run: |
2222
cd /usr/src
2323
apt-get update -y
24-
apt-get install -y wget
24+
apt-get install -y wget valgrind
2525
wget https://raw.githubusercontent.com/InterLinked1/phreakscript/refs/heads/master/phreaknet.sh
2626
chmod +x phreaknet.sh
2727
./phreaknet.sh make
@@ -39,20 +39,21 @@ jobs:
3939
/etc/init.d/asterisk restart
4040
sleep 3
4141
- name: Checkout
42-
uses: actions/checkout@v4
42+
uses: actions/checkout@v6
4343
- name: Build CAMI and execute simple demo
4444
run: |
4545
make
4646
make install
4747
make examples
4848
./simpleami
49+
valgrind --error-exitcode=1 ./simpleami
4950
fedora-42:
5051
runs-on: ubuntu-24.04
5152
name: Fedora 42
5253
container: fedora:42
5354
steps:
5455
- name: Checkout
55-
uses: actions/checkout@v4
56+
uses: actions/checkout@v6
5657
- name: Build CAMI
5758
run: |
5859
dnf install -y make gcc openssl-devel
@@ -65,7 +66,7 @@ jobs:
6566
container: fedora:42
6667
steps:
6768
- name: Checkout
68-
uses: actions/checkout@v4
69+
uses: actions/checkout@v6
6970
- name: Build CAMI
7071
run: |
7172
dnf install -y make gcc
@@ -77,7 +78,7 @@ jobs:
7778
runs-on: ubuntu-24.04
7879
name: FreeBSD
7980
steps:
80-
- uses: actions/checkout@v4
81+
- uses: actions/checkout@v6
8182
- name: Build CAMI
8283
uses: vmactions/freebsd-vm@v1
8384
with:

cami.c

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,26 +145,32 @@ static int maxwaitms = AMI_MAX_WAIT_TIME;
145145
static void *ami_event_dispatch(void *varg);
146146
static void ami_event_handle(struct ami_session *ami, char *data);
147147

148+
#define CLOSE(fd) close(fd); fd = -1;
149+
148150
static void close_pipes(struct ami_session *ami)
149151
{
150-
close(ami->ami_pipe[1]);
151-
close(ami->ami_read_pipe[1]);
152-
close(ami->ami_event_pipe[1]);
152+
CLOSE(ami->ami_pipe[1]);
153+
CLOSE(ami->ami_read_pipe[1]);
154+
CLOSE(ami->ami_event_pipe[1]);
153155

154-
close(ami->ami_pipe[0]);
155-
close(ami->ami_read_pipe[0]);
156-
close(ami->ami_event_pipe[0]);
156+
CLOSE(ami->ami_pipe[0]);
157+
CLOSE(ami->ami_read_pipe[0]);
158+
CLOSE(ami->ami_event_pipe[0]);
157159
}
158160

159161
static void ami_cleanup(struct ami_session *ami)
160162
{
161-
close_pipes(ami);
163+
/* ami_cleanup is called from both ami_disconnect and ami_loop,
164+
* so we ensure not to close the pipes twice. */
165+
if (ami->ami_event_pipe[0] != -1) {
166+
close_pipes(ami);
167+
}
162168

163169
/* Close read/write file descriptor, if it's the same.
164170
* If they are different, that means the application passed us file descriptors to use,
165171
* and we don't touch those. */
166172
if (ami->ami_wfd == ami->ami_rfd && ami->ami_rfd != -1) {
167-
close(ami->ami_rfd);
173+
CLOSE(ami->ami_rfd);
168174
ami->ami_rfd = -1;
169175
ami->ami_wfd = -1;
170176
}
@@ -503,7 +509,7 @@ static int common_init(struct ami_session *ami)
503509
pthread_mutex_init(&ami->ami_read_lock, NULL);
504510
pthread_mutex_lock(&ami->ami_read_lock);
505511

506-
#define CLOSE_PIPE_PAIR(pipepair) close(pipepair[0]); close(pipepair[1]);
512+
#define CLOSE_PIPE_PAIR(pipepair) CLOSE(pipepair[0]); CLOSE(pipepair[1]);
507513

508514
/* If we can't make a pipe, forget about the socket. */
509515
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, ami->ami_pipe)) {

0 commit comments

Comments
 (0)