Weird code crash

Lol! I thought cosmic rays when I read this thread.

Decades of having my nose rubbed in the s*it of my own stupidity, I guess. :-)

No, I do check it.

LOL!

Nope.

I am trying some stuff out to try and get it to fail *consistently*.

I dont feel its hugely profitable to attempt to debug it when most of the time its not doing anything wrong

The filenames never change length.

char filbuf[256]; char fullname[256];

The fullname is of the form

/var/www/data/volatile/192.168.0.xx.dat

There are no other files apart from 'relay.dat' in that directory.

I mean you are all throwing noob bugs at me. Yes, in 1984 that's the sort of s*it I used to write. Not these days.

I have a drawer full of T shirts marked 'buffer overrun' 'alloc without free' 'fopen without fclose'.

The fact is the memory footprint does not increase. So there are no obvious or simple memory leaks.

I've absolutely covered every error case mentioned here in the one case of the files that get written and read every few seconds.

It occurs to me that this behaviour started when I made it autoboot under systemd as well.

Since the consensus seems to be it isn't hardware, or file corruption, I am trying it launched manually to see if it crashes or not.

Systemd does seem to wrap things in resource limits, and start with a slightly different ENV although I cant see that any are being exceeded.

If it wasn't a daemon I would expect it to segfault and show that on screen. I could run it without daemonising it as well.

So lots of options to try.

As well as soft debuggers.

That cannot happen. Its hard wired into the code that writes the file

further up the line...

bzero(filbuf,sizeof(filbuf)); /** first thing to do is clean any allocated memory used to store values. **/ for(i=0;i<NUMBER_RELAYS;i++) free(thermometers[i].name);

Last resort. I have to learn how to *use* those tools. Right now I am working on other stuff and am content to change one thing at a time to see if that makes any difference.

That is a low user time strategy.

Thank you. The input has been valuable. And I now have further strategies in reserve.

As with all intermittent faults, the thing you need most is a reliable way to make the fault occur.

Oh, the thing I learned was that you should always put root as 1 and everything else as 2 ^^" but that makes more sense

You could get a SIGABRT if you were trying to free something that was already freed. Are you sure those are interlocked such that for each i you call strdup() exactly once, and subsequently free() exactly once? If there was some code path that was breaking out of the loop or similar you might get such behaviour.

Theo

Hmm. I free the pointers even for relay zones that don't have thermometers, whose pointers are 0. That isn't an issue.

But that might be a remotely possible issue. I dont zero the pointers after freeing them as far as I can tell. The silly thing is that this program doesn't use the name anyway.

Its used elsewhere Well I don't think its an issue, but I can zero the pointers anyway after free()ing

• The Natural Philosopher snipped-for-privacy@invalid.invalid | > | if(len=strncmp(filbuf,"ZONE",4)) //supposed to reject | > | a file whose contents do not start with ZONE | > | goto baddata; | > | | > | // looking very much like a temperature file | > | i=(int)filbuf[4] -'1'; // this is our zone from | > | "ZONE2" etc. 1-4 is zone but index is 0-3 so subtract | > | '1' | > The access of filbuf[4] is ok (since you checked that there are at | > least | > 4 characters in the file), but what if nothing follows after the 'ZONE', | > or ZONE is followed by anything but [1-4]?

| That cannot happen. Its hard wired into the code that writes the file

Depending on the permissions of VOLATILE_DIR, it *might* be possible that *anybody* can drop files in there. Save some "// skip known bollocks", you just scan every file in VOLATILE_DIR. If I were an attacker, I sure would try to use that vector, regardless whether the program in question runs with elevated permissions or not ;-)

| > Other than that, I really would have it running under a debugger or | > valgrind, since then *if* it crashes, you *know* *where* in your code it | > crashes. | > | Last resort. I have to learn how to *use* those tools.

With valgrind, it is as easy as putting 'valgrind' in front of the commandline you use to start your program. With gdb, it is a tiny bit more complicated, agreed. But since these tools are worth learning anyway for any programmer, the time invested in learning them is not wasted.

R'

• The Natural Philosopher snipped-for-privacy@invalid.invalid | > | thermometers[i].name=strdup(p); // | > | make a copy of the name and attach it | > | to our thermometer structure | > Memory leak if thermometers[i].name already contains something. | > | further up the line...

| bzero(filbuf,sizeof(filbuf)); | /** first thing to do is clean any allocated memory used to | store values. **/ | for(i=0;i<NUMBER_RELAYS;i++) | free(thermometers[i].name);

Note that the assignment

thermometers[i].name=strdup(p);

is *inside* the while() loop without a free().

Probably you argue that there ever is only a single file to read in that dir anyway... Personally, I've been bitten by such assumptions, so I'd rather check once too often than hunting down "can't happen" bugs.

R'

Well, I am not sure if that was it or not, but I deleted manually a thermometer file and the thing crashed instantly. That is consistent with the name having been set once, and then repeatedly free()ed. I then installed the code with the free()ed pointers set to NULL, and it

I had assumed that freeing a pointer that already had been freed would either result in a NO-OP because the pointer no longer existed in the heap memory allocation tables, or it would instantly crash , but it seems that the action is 'undefined'.

Not sure that's done the trick, because I don't quite see how a file could ever cease to exist.

To not exist in the first place is one thing, but once written, nothing should delete them.

Unless fopen("w") does that for a fraction of a microsecond

Or fopen("w") creates an *empty* file, in which case it is *just* possible that an empty file is read, no strdup was done and the pointer was double freed...next time around.

Academic now anyway. Pointers all set to null after freeing. Defined behaviour. frees on NULL ignored.

I'll let it run and run and see.

I thought double free was a SIGSEGV?

No. you have misunderstood how the code works.

There are up to 4 (NUMBER_RELAYS) thermometer files in that dir, and all of them are read in the loop. What there shouldn't be is more than one file with a ZONE number the same. So no pointer gets more than one STRDUP

If there were, it might be possible to strdup the same pointer twice. And the daemon would get a memory leak and crash.

(It would be trivial to simply add a conditional that only strdups to a pointer if it is NULL).

That is a possibility that could be caused by mis-configuration of the thermometers themselves.

However they are not at this time misconfigured, so it shouldn't be the crash problem, although it is an issue I will consider because fat fingers *could* cause it.

I do think that what has happened is that a valid file name has been found with empty data, or no file at all, and then no strdup is done - but the free is, next time around.

That should never happen of course, as the fopen/fwrite sequence should certainly not delete the filename, but it is entirely possible that a the fopen *truncates* its data. At which point we cant strdup anything, so the next free gets a woopsie

Setting the pointers to NULL after free() is nice defensive coding

As is allocating memory only if the pointers are null.

So both are in there now.

Yes, C explicitly labels "double free" as "undefined":

Look under J.2 Undefined behavior (easiest is to search for "free"):

J.2 Undefined behavior

...

The pointer argument to the free or realloc function does not match a pointer earlier returned by calloc, malloc, or realloc, or the space has been deallocated by a call to free or realloc (7.20.3.2, 7.20.3.4).

And th 7.20.3.2 link in the page jumps to this:

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by the calloc, malloc, or realloc function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

So if by chance you are double-freeing sometimes, then you are tickling the undefined behaviour devil, and all bets are off as to what might eventually occur.

Check my other reply to TNP for the details, but it is "undefined" in C.

In fact it seems fairly undefined

It looks like it is somewhat implementation dependent. SIGSEGV means you accessed unallocated memory, but that is not the same as freeing allocated memory, twice.

There seem to be instances of it reported. Google is a friend here.

I *suspect* that if that is the problem, its a signal from deep within libc. Whereas SIGSEGV probably emanates from a memory management unit somewhere

If Glibc detects it you’ll get a diagnostic and SIGABRT.

If it doesn’t detect it then anything could happen - SIGSEGV is just one possibility.

On Fri, 15 Sep 2023 14:56:23 +0100, The Natural Philosopher snipped-for-privacy@invalid.invalid wrote in <ue1nq7$39033$ snipped-for-privacy@dont-email.me:

Hi, read the thread with interest.

If you're getting SIGABRT, that's almost always the software calling abort(3). If you aren't, maybe there's a library calling it?

$ man 7 signal [...] Signal Standard Action Comment SIGABRT P1990 Core Abort signal from abort(3) [but it also lists] SIGIOT - Core IOT trap. A synonym for SIGABRT _ _ _ _ _ _ _

Meanwhile, if you want to avoid locking your file, you might want to write a fresh file with a unique name, then rename() it, which -- please correct me if I'm wrong -- should replace the desired file atomically.

Same here. Many years back I wrote the type of debugging and programming support library I personally find most useful: it can report the content of all common variable types as well as dumping byte arrays as both hex and ASCII as well as parsing the command line and allow the amount of debug info the be controlled by a command line argument.

They are structured as small libraries that designed to be lightweight enough to be left in a program when its in general use.

The library was originally written in C, but I soon wrote a Java version as well, though this hasn't been separately published yet.

If this sounds useful, both versions can be found on www.libelle- systems.com in the "Free Stuff" section.