From 86ffe4bec2d9c5f429dae3502e709ff7e730c710 Mon Sep 17 00:00:00 2001 From: Olaf Seibert Date: Sat, 15 Jul 2023 15:47:39 +0200 Subject: [PATCH] sim_disk.c: only free filebuf if it was allocated here too. PDP11/pdp11_rq.c re-uses ->filebuf as a sector buffer (under the #defined name rqxb) and allocates it as such. If an RQ disk is detached and another attached, this buffer would be lost and the pointer reset to NULL. sim_disk.c would only allocate the buffer if UNIT_BUFABLE is set, which means to buffer the whole disk. Since this rightly isn't set on RQ disks, the pointer would remain NULL and segfaults would ensue. See open-simh issue 274. add comment about safe re-use of filebuf --- PDP11/pdp11_rq.c | 1 + sim_disk.c | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/PDP11/pdp11_rq.c b/PDP11/pdp11_rq.c index c99b9141..39e8fe52 100644 --- a/PDP11/pdp11_rq.c +++ b/PDP11/pdp11_rq.c @@ -172,6 +172,7 @@ extern int32 MMR2; #define unit_plug u4 /* drive unit plug value */ #define io_status u5 /* io status from callback */ #define io_complete u6 /* io completion flag */ +/* we can re-use filebuf because we don't set UNIT_BUFABLE in flags */ #define rqxb filebuf /* xfer buffer */ #define RQ_RMV(u) ((drv_tab[GET_DTYPE (u->flags)].flgs & RQDF_RMV)? \ UF_RMV: 0) diff --git a/sim_disk.c b/sim_disk.c index a45ffd0c..4b9d0dfa 100644 --- a/sim_disk.c +++ b/sim_disk.c @@ -3380,12 +3380,14 @@ if ((uptr->flags & UNIT_BUF) && (uptr->filebuf)) { sim_messagef (SCPE_OK, "%s: writing buffer to file: %s\n", sim_uname (uptr), uptr->filename); sim_disk_wrsect (uptr, 0, (uint8 *)uptr->filebuf, NULL, (cap + ctx->sector_size - 1) / ctx->sector_size); } + if (uptr->flags & UNIT_MUSTBUF) { /* dyn alloc? */ + free (uptr->filebuf); /* free buffers */ + uptr->filebuf = NULL; + free (uptr->filebuf2); + uptr->filebuf2 = NULL; + } uptr->flags = uptr->flags & ~UNIT_BUF; } -free (uptr->filebuf); /* free buffers */ -uptr->filebuf = NULL; -free (uptr->filebuf2); -uptr->filebuf2 = NULL; update_disk_footer (uptr); /* Update meta data if highwater has changed */