code review 12493043: net: detect bad F_DUPFD_CLOEXEC on OS X 10.6

net: detect bad F_DUPFD_CLOEXEC on OS X 10.6

On 10.6, OS X's fcntl returns EBADF instead of EINVAL.

[bradfitz, 2013-08-06 00:04:10]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[iant, 2013-08-06 00:23:10]

According to build.golang.org the call is failing with EBADF, so suppose we just
try handling that the same way we handle EINVAL.

[bradfitz, 2013-08-06 00:25:25]

Maybe. But I don't want a real EBADF to neuter it for the lifetime of the
process, especially on systems that aren't OS X 10.6.

But maybe that can't happen.



On Mon, Aug 5, 2013 at 5:23 PM, <iant@golang.org> wrote:

> According to build.golang.org the call is failing with EBADF, so suppose
> we just try handling that the same way we handle EINVAL.
>
>
https://codereview.appspot.**com/12493043/<https://codereview.appspot.com/124...
>

[bradfitz, 2013-08-06 00:26:00]

I'm getting off the bus soon here, so somebody else can fix this if it's
urgent.



On Mon, Aug 5, 2013 at 5:25 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> Maybe. But I don't want a real EBADF to neuter it for the lifetime of the
> process, especially on systems that aren't OS X 10.6.
>
> But maybe that can't happen.
>
>
>
> On Mon, Aug 5, 2013 at 5:23 PM, <iant@golang.org> wrote:
>
>> According to build.golang.org the call is failing with EBADF, so suppose
>> we just try handling that the same way we handle EINVAL.
>>
>>
https://codereview.appspot.**com/12493043/<https://codereview.appspot.com/124...
>>
>
>

[iant, 2013-08-06 00:28:33]

On Mon, Aug 5, 2013 at 5:25 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> Maybe. But I don't want a real EBADF to neuter it for the lifetime of the
> process, especially on systems that aren't OS X 10.6.
>
> But maybe that can't happen.

I don't think it can happen in a normal program that isn't doing
something broken like calling File() on a net.TCPListener that was
closed.

Ian

> On Mon, Aug 5, 2013 at 5:23 PM, <iant@golang.org> wrote:
>>
>> According to build.golang.org the call is failing with EBADF, so suppose
>> we just try handling that the same way we handle EINVAL.
>>
>> https://codereview.appspot.com/12493043/
>
>

[dave_cheney.net, 2013-08-06 01:30:16]

I have a darwin/386 system here, let me test.



On 06/08/2013, at 10:25, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> Maybe. But I don't want a real EBADF to neuter it for the lifetime of the
process, especially on systems that aren't OS X 10.6.
> 
> But maybe that can't happen.
> 
> 
> 
> On Mon, Aug 5, 2013 at 5:23 PM, <iant@golang.org> wrote:
>> According to build.golang.org the call is failing with EBADF, so suppose
>> we just try handling that the same way we handle EINVAL.
>> 
>> https://codereview.appspot.com/12493043/
> 
> -- 
>  
> --- 
> You received this message because you are subscribed to the Google Groups
"golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>  
>  

[bradfitz, 2013-08-06 01:33:12]

I just booted my 10.6 machine.  It was running pre-Go 1 go from Dec 21,
2011.

But I can reproduce the failure now too.



On Mon, Aug 5, 2013 at 6:29 PM, Dave Cheney <dave@cheney.net> wrote:

> I have a darwin/386 system here, let me test.
>
>
>
> On 06/08/2013, at 10:25, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>
> Maybe. But I don't want a real EBADF to neuter it for the lifetime of the
> process, especially on systems that aren't OS X 10.6.
>
> But maybe that can't happen.
>
>
>
> On Mon, Aug 5, 2013 at 5:23 PM, <iant@golang.org> wrote:
>
>> According to build.golang.org the call is failing with EBADF, so suppose
>> we just try handling that the same way we handle EINVAL.
>>
>>
https://codereview.appspot.**com/12493043/<https://codereview.appspot.com/124...
>>
>
>  --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>
>

[dave_cheney.net, 2013-08-06 02:18:40]

LGTM. Passes on my darwin/386 machine.

[bradfitz, 2013-08-06 03:39:54]

I downloaded
http://www.opensource.apple.com/tarballs/xnu/xnu-1504.15.3.tar.gz (the
kernel for 10.6 on my machine at least) and read
bsd/kern/kern_descrip.c's fcntl_nocancel.

Looks like we're hitting:

default:
                /*
                 * This is an fcntl() that we d not recognize at this level;
                 * if this is a vnode, we send it down into the VNOP_IOCTL
                 * for this vnode; this can include special devices, and
will
                 * effectively overload fcntl() to send ioctl()'s.
                 */
                if((uap->cmd & IOC_VOID) && (uap->cmd & IOC_INOUT)){
                        error = EINVAL;
                        goto out;
                }

                if (fp->f_type != DTYPE_VNODE) {
                        error = EBADF;
                        goto out;
                }


And sure enough, in xnu-1699 (for 10.7) and xnu-2050 (for 10.8), I see:

        switch (uap->cmd) {

        case F_DUPFD:
        case F_DUPFD_CLOEXEC:

In any case, at least I understand the EBADF now.  The 10.6 fcntl(2)
manpage says nothing about that fallback-to-ioctl behavior, of course, nor
the EBADF-on-bogus-cmd.




On Mon, Aug 5, 2013 at 7:18 PM, <dave@cheney.net> wrote:

> LGTM. Passes on my darwin/386 machine.
>
>
https://codereview.appspot.**com/12493043/<https://codereview.appspot.com/124...
>

[bradfitz, 2013-08-06 03:58:35]

Hello golang-dev@googlegroups.com, iant@golang.org, dave@cheney.net (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-08-06 03:59:07]

PTAL.  Slightly tweaked, no more init func, and better comment.



On Mon, Aug 5, 2013 at 8:58 PM, <bradfitz@golang.org> wrote:

> Hello golang-dev@googlegroups.com, iant@golang.org, dave@cheney.net (cc:
> golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
https://codereview.appspot.**com/12493043/<https://codereview.appspot.com/124...
>

[dave_cheney.net, 2013-08-06 04:27:11]

Testing now. 

On 06/08/2013, at 13:59, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> PTAL.  Slightly tweaked, no more init func, and better comment.
> 
> 
> 
> On Mon, Aug 5, 2013 at 8:58 PM, <bradfitz@golang.org> wrote:
>> Hello golang-dev@googlegroups.com, iant@golang.org, dave@cheney.net (cc:
>> golang-dev@googlegroups.com),
>> 
>> Please take another look.
>> 
>> 
>> https://codereview.appspot.com/12493043/
> 

[iant, 2013-08-06 05:14:38]

LGTM

[dave_cheney.net, 2013-08-06 05:32:13]

LGTM. Thanks.

[bradfitz, 2013-08-06 14:18:21]

*** Submitted as https://code.google.com/p/go/source/detail?r=2585605c42c0 ***

net: detect bad F_DUPFD_CLOEXEC on OS X 10.6

On 10.6, OS X's fcntl returns EBADF instead of EINVAL.

R=golang-dev, iant, dave
CC=golang-dev
https://codereview.appspot.com/12493043