Skip to content
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

Jump to bottom

Fix two problems with unset -f behavior #22

Merged
merged 1 commit into from
Jun 17, 2020
Merged

Fix two problems with unset -f behavior #22

merged 1 commit into from
Jun 17, 2020

Conversation

JohnoKing
Copy link

@JohnoKing JohnoKing commented Jun 17, 2020 •
edited
Loading

This pull request backports two bug fixes from ksh93v- for the problems detailed in #21. These fixes do the following:

In src/cmd/ksh93/sh/name.c, the check for if a function is still running has been corrected to fix a segmentation fault that occurred when a POSIX function unset itself.

The missing functionality for when a KornShell style function unsets itself has been added to src/cmd/ksh93/sh/xec.c. This kind of function will now be properly unset after it has finished running, rather than silently cause unset -f to fail.

McDutchie
McDutchie requested changes Jun 17, 2020
View reviewed changes
Copy link

@McDutchie McDutchie left a comment

Choose a reason for hiding this comment

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

I was half expecting you to show up for this one. :) Couple of comments below…

@JohnoKing
Fix two problems with unset -f behavior
38d9279 
src/cmd/ksh93/sh/name.c:
 - Correct the check for when a function is currently running
   to fix a segmentation fault that occurred when a POSIX
   function tries to unset itself while it is running.
   This bug fix was backported from ksh93v-.

src/cmd/ksh93/sh/xec.c:
 - If a function tries to unset itself, unset the function
   with `_nv_unset(np, NV_RDONLY)` to fix a silent failure.
   This fix was also backported from ksh93v-.

src/cmd/ksh93/tests/functions.sh:
 - Add four regression tests for when a function unsets itself.
@McDutchie McDutchie merged commit b7932e8 into ksh93:master Jun 17, 2020
@McDutchie McDutchie mentioned this pull request Jun 17, 2020
Closed
McDutchie added a commit that referenced this pull request Jun 17, 2020
@McDutchie
Part revert #22 to undo memory fault (re: b7932e8)
9751174 
The fix in sh/xec.c, which was backported from the ksh 93v- beta to
delay the actual removal of a running function that unsets itself,
caused a segfault in the variables.sh regression tests (see #23).

src/cmd/ksh93/sh/xec.c:
- Comment out the backported code pending a correct fix for #21.
  Now both types of functions silently fail to unset themselves
  (unless they're discipline functions).

src/cmd/ksh93/tests/functions.sh:
- Disable regression tests checking that the function was actually
  unset, pending a correct fix for #21.

Resolves: #23
Reopens: #21
McDutchie added a commit that referenced this pull request Jun 18, 2020
@McDutchie
Restore #22 'unset -f' fix minus segfault (re: b7932e8, 9751174)
3e3f6b0 
Applying the fix for 'unset -f' exposed a crashing bug in lookup()
in sh/nvdisc.c, which is the function for looking up discipline
functions. This is what caused tests/variables.sh to crash.
Ref.: #23 (comment)

src/cmd/ksh93/sh/nvdisc.c: lookup():
- To avoid segfault, check that the function pointer nq->nvalue.rp
  is actually set before checking if nq->nvalue.rp->running==1.

src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/tests/functions.sh:
- Uncomment the 'unset -f' fix from b7932e8.

Resolves #21 (again).
@JohnoKing JohnoKing deleted the fix-unset-f branch June 18, 2020 02:45
JohnoKing added a commit to JohnoKing/ksh that referenced this pull request Sep 23, 2022
@JohnoKing
Fix buffer overflow in quoting2.sh under ASan
d97873a 
The isaname, isaletter, isadigit, isexp and ismeta macros don't check if
c is a negative value before accessing sh_lexstates. This can result in
ASan crashing because of a buffer overflow in quoting2.sh when running
in a multibyte locale:
  test quoting2(C.UTF-8) begins at 2022-09-23+14:03:12
  =================================================================
  ==262224==ERROR: AddressSanitizer: global-buffer-overflow on address 0x557b201a451f at pc 0x557b1fe5e6fc bp 0x7fffcf1ac700 sp 0x7fffcf1ac6f8
  READ of size 1 at 0x557b201a451f thread T0
      #0 0x557b1fe5e6fb in sh_fmtq /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/string.c:341:5
      ksh93#1 0x557b1fe6098c in sh_fmtqf /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/string.c:473:10
      ksh93#2 0x557b1ff08dc0 in extend /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:998:14
      ksh93#3 0x557b2008a56c in sfvprintf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/sfio/sfvprintf.c:531:8
      ksh93#4 0x557b2005b7f7 in sfprintf /home/johno/GitRepos/KornShell/ksh/src/lib/libast/sfio/sfprintf.c:31:7
      ksh93#5 0x557b1ff04272 in b_print /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:343:4
      ksh93#6 0x557b1ff04ebf in b_printf /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/print.c:148:9
      ksh93#7 0x557b1fe8d9a7 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1261:21
      ksh93#8 0x557b1fe7a7cf in sh_subshell /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/subshell.c:652:4
      ksh93#9 0x557b1fdedc0d in comsubst /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:2207:9
      ksh93#10 0x557b1fdefc79 in varsub /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:1181:3
      ksh93#11 0x557b1fde3bef in copyto /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:620:21
      ksh93#12 0x557b1fde0b07 in sh_mactrim /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/macro.c:169:2
      ksh93#13 0x557b1fe05ab6 in nv_setlist /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/name.c:280:9
      ksh93#14 0x557b1fe8a7e8 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1051:7
      ksh93#15 0x557b1fe95b85 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1940:5
      ksh93#16 0x557b1fe99ea6 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:2271:10
      ksh93#17 0x557b1fd23b04 in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604:4
      ksh93#18 0x557b1fd1fe10 in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369:2
      ksh93#19 0x557b1fd1d585 in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41:9
      ksh93#20 0x7f55d5b5028f  (/usr/lib/libc.so.6+0x2328f) (BuildId: 26c81e7e05ebaf40bac3523b7d76be0cd71fad82)
      ksh93#21 0x7f55d5b50349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) (BuildId: 26c81e7e05ebaf40bac3523b7d76be0cd71fad82)
      ksh93#22 0x557b1fc158d4 in _start /build/glibc/src/glibc/csu/../sysdeps/x86_64/start.S:115

src/cmd/ksh93/include/lexstates.h:
- Check if c is negative before accessing sh_lexstates. Backported from
  ksh2020: att@a7013320.
  I'll note that later in ksh2020 these macros became functions:
  att@adc589de. I didn't backport that
  commit because it requires the C99 bool type to avoid compiler
  warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@McDutchie McDutchie McDutchie requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JohnoKing @McDutchie