* Re: New Defects reported by Coverity Scan for zsh
[not found] ` <CAH+w=7ZeUFAP3kpFUxuagz9iv+a98SAr4EQ4iXAFvEZBuXc_2A@mail.gmail.com>
@ 2023-03-01 3:13 ` Bart Schaefer
2023-03-01 3:50 ` Mikael Magnusson
0 siblings, 1 reply; 2+ messages in thread
From: Bart Schaefer @ 2023-03-01 3:13 UTC (permalink / raw)
To: Zsh hackers list
Why am I getting this? Who set this up?
On Tue, Feb 28, 2023 at 5:01 AM <scan-admin@coverity.com> wrote:
>
> >>> CID 1521554: Control flow issues (MISSING_RESTORE)
> >>> Value of non-local "*ss" that was saved in "sav" is not restored as it was along other paths.
> 2159 return NULL;
Pointer to heap memory, not used again, no need to restore.
> /Src/params.c: 6268 in upscope()
> >>> Null-checking "pm" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
Caller shouldn't ever pass NULL. Is this going to keep complaining about it?
> *** CID 1521548: Memory - illegal accesses (USE_AFTER_FREE)
> /Src/builtin.c: 1211 in cd_new_pwd()
> 1205 zsfree(getlinknode(dirstack));
> 1206
> 1207 if (chasinglinks) {
> 1208 s = findpwd(new_pwd);
> 1209 if (s) {
> 1210 zsfree(new_pwd);
> >>> CID 1521548: Memory - illegal accesses (USE_AFTER_FREE)
> >>> Using freed pointer "s".
> 1211 new_pwd = s;
> 1212 }
This is a knock-on to the complaint about findpwd() below.
> 7181 if (meta) {
> >>> CID 1521546: Uninitialized variables (UNINIT)
> >>> Using uninitialized value "t[-1]".
> 7182 t[-1] |= 0x80;
> 7183 meta = 0;
> 7184 }
Hm, I guess "t" might not have advanced past its original starting
assignment if control passes through the #ifdef MULTIBYTE block about
60 lines earlier, without returning?
#ifdef MULTIBYTE_SUPPORT
} else if ((how & GETKEY_SINGLE_CHAR) &&
isset(MULTIBYTE) && (unsigned char) *s > 127) {
wint_t wc;
int len;
len = mb_metacharlenconv(s, &wc);
if (wc != WEOF) {
*misc = (int)wc;
return s + len;
}
#endif
> *** CID 1521545: Resource leaks (RESOURCE_LEAK)
> /Src/Modules/param_private.c: 130 in makeprivate()
> >>> CID 1521545: Resource leaks (RESOURCE_LEAK)
> >>> Variable "gsu" going out of scope leaks the storage it points to.
Can't happen unless the definition of PM_TYPE() changes without this
code being updated.
> *** CID 1521544: Memory - illegal accesses (USE_AFTER_FREE)
> /Src/utils.c: 801 in findpwd()
> 795
> 796 if (*s == '/')
> 797 return xsymlink(s, 0);
> 798 s = tricat((pwd[1]) ? pwd : "", "/", s);
> 799 t = xsymlink(s, 0);
> 800 zsfree(s);
> >>> CID 1521544: Memory - illegal accesses (USE_AFTER_FREE)
> >>> Using freed pointer "t".
> 801 return t;
> 802 }
Not seeing how it calculates this one, I think xsymlink(s,0) is going
to end up returning either a pointer to the static mbuf[] in metafy(),
or heap memory. Anyone else see an alternative? Is it treating mbuf
as freed stack even though it is declared static?
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: New Defects reported by Coverity Scan for zsh
2023-03-01 3:13 ` New Defects reported by Coverity Scan for zsh Bart Schaefer
@ 2023-03-01 3:50 ` Mikael Magnusson
0 siblings, 0 replies; 2+ messages in thread
From: Mikael Magnusson @ 2023-03-01 3:50 UTC (permalink / raw)
To: Bart Schaefer; +Cc: Zsh hackers list
On 3/1/23, Bart Schaefer <schaefer@brasslantern.com> wrote:
> Why am I getting this? Who set this up?
Presumably at some point you joined the project on coverity (you are
listed as an admin there, I don't think I could have added you against
your will). I think I originally set it up, but I've not run the build
for just over a year. I did today, so that's why you got the little
mail notification.
> On Tue, Feb 28, 2023 at 5:01 AM <scan-admin@coverity.com> wrote:
>>
>> >>> CID 1521554: Control flow issues (MISSING_RESTORE)
>> >>> Value of non-local "*ss" that was saved in "sav" is not restored
>> >>> as it was along other paths.
>> 2159 return NULL;
>
> Pointer to heap memory, not used again, no need to restore.
>
>> /Src/params.c: 6268 in upscope()
>> >>> Null-checking "pm" suggests that it may be null, but it has
>> >>> already been dereferenced on all paths leading to the check.
>
> Caller shouldn't ever pass NULL. Is this going to keep complaining about
> it?
These mails ideally only include new issues (sometimes if code changes
a bit, it might not realize it's the same thing and include it again),
but it will be listed on the site until someone either changes the
code or marks it as Ignore on the site.
>> *** CID 1521548: Memory - illegal accesses (USE_AFTER_FREE)
>> /Src/builtin.c: 1211 in cd_new_pwd()
>> 1205 zsfree(getlinknode(dirstack));
>> 1206
>> 1207 if (chasinglinks) {
>> 1208 s = findpwd(new_pwd);
>> 1209 if (s) {
>> 1210 zsfree(new_pwd);
>> >>> CID 1521548: Memory - illegal accesses (USE_AFTER_FREE)
>> >>> Using freed pointer "s".
>> 1211 new_pwd = s;
>> 1212 }
>
> This is a knock-on to the complaint about findpwd() below.
>
>> 7181 if (meta) {
>> >>> CID 1521546: Uninitialized variables (UNINIT)
>> >>> Using uninitialized value "t[-1]".
>> 7182 t[-1] |= 0x80;
>> 7183 meta = 0;
>> 7184 }
>
> Hm, I guess "t" might not have advanced past its original starting
> assignment if control passes through the #ifdef MULTIBYTE block about
> 60 lines earlier, without returning?
>
> #ifdef MULTIBYTE_SUPPORT
> } else if ((how & GETKEY_SINGLE_CHAR) &&
> isset(MULTIBYTE) && (unsigned char) *s > 127) {
> wint_t wc;
> int len;
> len = mb_metacharlenconv(s, &wc);
> if (wc != WEOF) {
> *misc = (int)wc;
> return s + len;
> }
> #endif
>
>
>> *** CID 1521545: Resource leaks (RESOURCE_LEAK)
>> /Src/Modules/param_private.c: 130 in makeprivate()
>> >>> CID 1521545: Resource leaks (RESOURCE_LEAK)
>> >>> Variable "gsu" going out of scope leaks the storage it points to.
>
> Can't happen unless the definition of PM_TYPE() changes without this
> code being updated.
>
>> *** CID 1521544: Memory - illegal accesses (USE_AFTER_FREE)
>> /Src/utils.c: 801 in findpwd()
>> 795
>> 796 if (*s == '/')
>> 797 return xsymlink(s, 0);
>> 798 s = tricat((pwd[1]) ? pwd : "", "/", s);
>> 799 t = xsymlink(s, 0);
>> 800 zsfree(s);
>> >>> CID 1521544: Memory - illegal accesses (USE_AFTER_FREE)
>> >>> Using freed pointer "t".
>> 801 return t;
>> 802 }
>
> Not seeing how it calculates this one, I think xsymlink(s,0) is going
> to end up returning either a pointer to the static mbuf[] in metafy(),
> or heap memory. Anyone else see an alternative? Is it treating mbuf
> as freed stack even though it is declared static?
--
Mikael Magnusson
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-03-01 3:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <63fdfb42fe26_9c8392b226e1f79b07046a@prd-scan-dashboard-0.mail>
[not found] ` <CAH+w=7ZeUFAP3kpFUxuagz9iv+a98SAr4EQ4iXAFvEZBuXc_2A@mail.gmail.com>
2023-03-01 3:13 ` New Defects reported by Coverity Scan for zsh Bart Schaefer
2023-03-01 3:50 ` Mikael Magnusson
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).