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

Reply via email to