Skip to content

Commit

Permalink
Fix crash on failure to trim ~/.sh_history
Browse files Browse the repository at this point in the history
@vmihalko writes:
> We were able to reproduce an old issue mentioned in
> https://bugzilla.redhat.com/show_bug.cgi?id=1885399 using the
> latest version of ksh. The corresponding code has not changed
> much in the past few years.
>
> To provide further explanation, the problem arises when a user's
> .sh_history file grows to a size that triggers the hist_trim
> function, but the user lacks (after the creation of .sh_history)
> the necessary write permissions to their $HOME directory. As a
> result, ksh becomes stuck in a recursive loop between the
> sh_histinit(src/cmd/ksh93/edit/history.c#L203) function and the
> hist_trim(src/cmd/ksh93/edit/history.c#L417) function.
>
> Conditions for reproduction:
>
> 1. The size of the .sh_history file is larger than the HIST_MAX
>    limit. (src/cmd/ksh93/edit/history.c, line 325)
> 2. .sh_history file has not been changed in the HIST_RECENT
>    seconds (src/cmd/ksh93/edit/history.c, line 406)
> 3. The user does not have permission to write to the $HOME
>    directory.

src/cmd/ksh93/edit/history.c: hist_trim():
- Print a warning and return if unlink(2) fails. The warning tells
  the user to check the history file's parent directory is
  writable. This is the best I realistically do for now, because
  this function's basic method assumes a writable parent directory.
- The temp file fallback is deleted because it's fundamentally
  flawed: it assumes the temp file is made on the same volume as
  the history file and can simply be rename(2)'d in place. Even
  on systems where this is the case, it doesn't appear to be
  working correctly, but this is not worth looking into.

Resolves: #695
  • Loading branch information
McDutchie committed Dec 28, 2023
1 parent b1e514c commit 2075b2b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 30 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
2023-09-15, that occurred on some systems when running an external command
with a redirection from a command substitution.

- Fixed an init-time crash on failure to trim the shell command history file
due to a non-writable parent directory; ksh now prints a warning instead.

2023-12-25:

- Fixed a regression in the behavior of 'exit' in a trap action, introduced
Expand Down
34 changes: 4 additions & 30 deletions src/cmd/ksh93/edit/history.c
Original file line number Diff line number Diff line change
Expand Up @@ -419,34 +419,13 @@ static History_t* hist_trim(History_t *hp, int n)
char *cp;
int incmd=1, c=0;
History_t *hist_new, *hist_old = hp;
char *buff, *endbuff, *tmpname=0;
char *buff, *endbuff;
off_t oldp,newp;
struct stat statb;
unlink(hist_old->histname);
if(access(hist_old->histname,F_OK) >= 0)
if(unlink(hist_old->histname) < 0)
{
/* The unlink can fail on Windows 95 */
int fd;
char *last, *name=hist_old->histname;
sh_close(sffileno(hist_old->histfp));
tmpname = (char*)sh_malloc(strlen(name)+14);
if(last = strrchr(name,'/'))
{
*last = 0;
pathtmp(tmpname,name,"hist",NULL);
*last = '/';
}
else
pathtmp(tmpname,e_dot,"hist",NULL);
if(rename(name,tmpname) < 0)
{
free(tmpname);
tmpname = name;
}
fd = open(tmpname,O_RDONLY|O_cloexec);
sfsetfd(hist_old->histfp,fd);
if(tmpname==name)
tmpname = 0;
errormsg(SH_DICT,ERROR_warn(0),"cannot trim history file %s; make sure parent directory is writable",hist_old->histname);
return hist_ptr = hist_old;
}
hist_ptr = 0;
if(fstat(sffileno(hist_old->histfp),&statb)>=0)
Expand Down Expand Up @@ -501,11 +480,6 @@ static History_t* hist_trim(History_t *hp, int n)
}
hist_cancel(hist_new);
sfclose(hist_old->histfp);
if(tmpname)
{
unlink(tmpname);
free(tmpname);
}
free((char*)hist_old);
return hist_ptr = hist_new;
}
Expand Down

0 comments on commit 2075b2b

Please sign in to comment.