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.

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

pkg/ipc: makeCommand calls inwp.Close twice #897

Closed
jackseans2012 opened this issue Dec 26, 2018 · 9 comments

Comments

Projects
None yet
2 participants
@jackseans2012
Copy link

commented Dec 26, 2018

in ipc.go:534
defer inwp.Close()
in ipc.go:592
inwp.Close()

@dvyukov

This comment has been minimized.

Copy link
Collaborator

commented Dec 26, 2018

What problem does it cause? AFAIR it is intentional. defer to not leak it, explicit close before handshake needed for something.

@dvyukov dvyukov changed the title makeCommand() pkg/ipc/ipc.go call inwp.close() twice pkg/ipc: makeCommand calls inwp.Close twice Dec 26, 2018

@jackseans2012

This comment has been minimized.

Copy link
Author

commented Dec 26, 2018

efer func (){
log.Logf(0,"[*]ipc.makecommand.inwp.close2\n")
inwp.Close()
}()

   wp.Close()
    log.Logf(0,"[*]ipc.makecommand.inwp.close1\n")
    inwp.Close()

2018/12/26 09:10:24 []ipc.makecommand.inwp.close1
2018/12/26 09:10:24 [
]ipc.makecommand.inwp.close2
so i think don't need close1

@dvyukov

This comment has been minimized.

Copy link
Collaborator

commented Dec 26, 2018

AFAIR it is required for handshake. If the pipe is not closed, the process does not terminate or something like that. I mean if we just remove close2 it can change program behavior. Are you sure it won't?

@jackseans2012

This comment has been minimized.

Copy link
Author

commented Dec 27, 2018

i remove close1 .it work ok.

@jackseans2012

This comment has been minimized.

Copy link
Author

commented Dec 27, 2018

inwp = OS.pipe() in kernel, it's file->f_cout 1
cmd.Start() ,sys_clone will inc f_cout ,and dup2 also inc f_cout
so the f_cout is 3

@dvyukov

This comment has been minimized.

Copy link
Collaborator

commented Dec 27, 2018

i remove close1 .it work ok.

But... if we remove the first one, it will cause resource leaks on errors.

@dvyukov

This comment has been minimized.

Copy link
Collaborator

commented Dec 27, 2018

inwp = OS.pipe() in kernel, it's file->f_cout 1
cmd.Start() ,sys_clone will inc f_cout ,and dup2 also inc f_cout
so the f_cout is 3

I don't understand what you mean here.

@jackseans2012

This comment has been minimized.

Copy link
Author

commented Dec 27, 2018

i remove close1 .it work ok.

But... if we remove the first one, it will cause resource leaks on errors.

inwp.close is close 1 and defer inwp.close is close2

@jackseans2012

This comment has been minimized.

Copy link
Author

commented Dec 27, 2018

inwp = OS.pipe() in kernel, it's file->f_cout 1
cmd.Start() ,sys_clone will inc f_cout ,and dup2 also inc f_cout
so the f_cout is 3

I don't understand what you mean here.
when pipe is created in linux kerenel. the struct file- >f_count is pipe's reference count
close() will dec f_count
cmd.Start() will call sys_clone --> sys_dup2(inwp,1) -- > sys_exec --> syz-executor
so clone will call fget(inwp) , cause inc f_count,and dup2 also inc f_count

@dvyukov dvyukov closed this in 8e3d1cb Dec 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.