On Thu, 26 Aug 2010 14:34:09 +0200
Tomáš Smetana <[email protected]> wrote:
> Hello,
> while debugging a segfault in sfswap I noticed the following in the
> io.c:sh_iorestore() function:
>
> 1662 ⋅ ⋅ if ((savefd = filemap[fd].save_fd) >= 0)
> 1663 ⋅ ⋅ {
> 1664 ⋅ ⋅ ⋅ sh_fcntl(savefd, F_DUPFD, origfd);
> 1665 ⋅ ⋅ ⋅ if(savefd==job.fd)
> 1666 ⋅ ⋅ ⋅ ⋅ job.fd=origfd;
> 1667 ⋅ ⋅ ⋅ shp->fdstatus[origfd] = shp->fdstatus[savefd];
> 1668 ⋅ ⋅ ⋅ /* turn off close-on-exec if flag if necessary */
> 1669 ⋅ ⋅ ⋅ if(shp->fdstatus[origfd]&IOCLEX)
> 1670 ⋅ ⋅ ⋅ ⋅ fcntl(origfd,F_SETFD,FD_CLOEXEC);
> 1671 ⋅ ⋅ ⋅ if(origfd<=2)
> 1672 ⋅ ⋅ ⋅ {
> 1673 ⋅ ⋅ ⋅ ⋅ sfswap(shp->sftable[savefd],shp->sftable[origfd]);
> 1674 ⋅ ⋅ ⋅ ⋅ if(origfd==0)
> 1675 ⋅ ⋅ ⋅ ⋅ ⋅ shp->st.ioset = 0;
> 1676 ⋅ ⋅ ⋅ }
> 1677 ⋅ ⋅ ⋅ else
> 1678 ⋅ ⋅ ⋅ ⋅ shp->sftable[origfd] = shp->sftable[savefd];
> 1679 ⋅ ⋅ ⋅ shp->sftable[savefd] = 0;
> 1680 ⋅ ⋅ ⋅ sh_close(savefd);
> 1681 ⋅ ⋅ }
>
> The segfault in my case came from the problem when origfd value has been set
> to -1 (F_DUPFD failed) and then accessing shp->sftable[origfd] lead to
> crash. I believe the root cause for the problem I'm investigating is
> somewhere else but not checking the origfd value after the sh_fcntl call on
> line 1664 and missing error handling doesn't seem to be correct to me (the
> fcntl call simply can fail and we should count with that).
>
> Am I right?
No and It's actually difficult to remember how did I come the what I have
written above... It's completely wrong (maybe not completely -- I still think
not checking the sh_fcntl return code is not quite OK).
Sorry for the noise,
--
Tomáš Smetana
_______________________________________________
ast-developers mailing list
[email protected]
https://mailman.research.att.com/mailman/listinfo/ast-developers