Backport #5075: invalid *fdp in Mac OS X and FreeBSD over recvmsg with SCM_RIGHTS

• Status: Rejected
• Assignee: yugui (Yuki Sonoda)

[naruse (Yui NARUSE)]

Mac OS X と FreeBSD にて、存在しない fd を close してしまう問題について、
現在 r32598 で応急処置が施されていますが、根本的な原因について、
sys/kern/uipc_socket.c を見るに、
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c?rev=1.340.2.6.2.1;content-type=text%2Fplain;only_with_tag=RELENG_8_2_0_RELEASE

 * Process one or more MT_CONTROL mbufs present before any data mbufs
 * in the first mbuf chain on the socket buffer.  If MSG_PEEK, we
 * just copy the data; if !MSG_PEEK, we call into the protocol to
 * perform externalization (or freeing if controlp == NULL).

とあるので、recvmsg に MSG_PEEK を与えた場合は invalid なものが返ってくると思うのですが。

ちなみに、以下のような printf パッチをあてて走らせると、discard_cmsg() に来たものは全て invalid になっています。

diff --git a/ext/socket/ancdata.c b/ext/socket/ancdata.c
index 61e0576..ad44fb4 100644
--- a/ext/socket/ancdata.c
+++ b/ext/socket/ancdata.c
@@ -1379,6 +1379,7 @@ rb_recvmsg(int fd, struct msghdr *msg, int flags)
static void
discard_cmsg(struct cmsghdr *cmh, char *msg_end)
{

• fprintf(stderr, "discard_cmsg-begin\n");
  if (cmh->cmsg_level == SOL_SOCKET && cmh->cmsg_type == SCM_RIGHTS) {
  int *fdp = (int *)CMSG_DATA(cmh);
  int *end = (int *)((char *)cmh + cmh->cmsg_len);
  @@ -1391,12 +1392,18 @@ discard_cmsg(struct cmsghdr *cmh, char *msg_end)
  */
  struct stat buf;
  if (fstat(*fdp, &buf) == 0) {

•       fprintf(stderr, "fdp: %d is valid   (%p %p %p)\n", *fdp,fdp,end,msg_end);
             rb_update_max_fd(*fdp);
             close(*fdp);
         }
  

•       else {
  

•           fprintf(stderr, "fdp: %d is invalid (%p %p %p)\n", *fdp,fdp,end,msg_end);
  

•           rb_backtrace();
  

•       }
         fdp++;
     }
  

  }
• fprintf(stderr, "discard_cmsg-end\n");
  }
  #endif

@@ -1432,6 +1439,7 @@ make_io_for_unix_rights(VALUE ctl, struct cmsghdr *cmh, char *msg_end)
(char *)fdp + sizeof(int) <= msg_end) {
int fd = *fdp;
struct stat stbuf;

• fprintf(stderr,"makeiounixr: %d (%p %p %p)\n", *fdp,fdp,end,msg_end);
  VALUE io;
  if (fstat(fd, &stbuf) == -1)
  rb_raise(rb_eSocket, "invalid fd in SCM_RIGHTS");

[kosaki (Motohiro KOSAKI)]

xnu/bsd/kern/uipc_socket.c に全く同じコメントがあるので、FreeBSD由来のコードで同じ問題にあたってるんじゃないですかね。

[akr (Akira Tanaka)]

• File recvmsg-msg_peek-freebsd.patch recvmsg-msg_peek-freebsd.patch added

[akr (Akira Tanaka)]

• ruby -v changed from ruby 1.9.4dev (2011-07-22 trunk 32604) [x86_64-freebsd8.2] to -

2011/7/22 Yui NARUSE [e-mail]:

Mac OS X と FreeBSD にて、存在しない fd を close してしまう問題について、
現在 r32598 で応急処置が施されていますが、根本的な原因について、
sys/kern/uipc_socket.c を見るに、
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c?rev=1.340.2.6.2.1;content-type=text%2Fplain;only_with_tag=RELENG_8_2_0_RELEASE

    * Process one or more MT_CONTROL mbufs present before any data mbufs
    * in the first mbuf chain on the socket buffer.  If MSG_PEEK, we
    * just copy the data; if !MSG_PEEK, we call into the protocol to
    * perform externalization (or freeing if controlp == NULL).

とあるので、recvmsg に MSG_PEEK を与えた場合は invalid なものが返ってくると思うのですが。

おぉ、素晴らしい。

ではとりあえず FreeBSD と MacOS X で MSG_PEEK のとき、という条件ですかね。

[田中 哲][たなか あきら][Tanaka Akira]

[akr (Akira Tanaka)]

2011/7/22 Yui NARUSE [e-mail]:

Mac OS X と FreeBSD にて、存在しない fd を close してしまう問題について、
現在 r32598 で応急処置が施されていますが、根本的な原因について、
sys/kern/uipc_socket.c を見るに、
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c?rev=1.340.2.6.2.1;content-type=text%2Fplain;only_with_tag=RELENG_8_2_0_RELEASE

    * Process one or more MT_CONTROL mbufs present before any data mbufs
    * in the first mbuf chain on the socket buffer.  If MSG_PEEK, we
    * just copy the data; if !MSG_PEEK, we call into the protocol to
    * perform externalization (or freeing if controlp == NULL).

とあるので、recvmsg に MSG_PEEK を与えた場合は invalid なものが返ってくると思うのですが。

おぉ、素晴らしい。

ではとりあえず FreeBSD と MacOS X で MSG_PEEK のとき、という条件ですかね。

[田中 哲][たなか あきら][Tanaka Akira]

[kosaki (Motohiro KOSAKI)]

Issue #5075 has been updated by Akira Tanaka.

File recvmsg-msg_peek-freebsd.patch added

わたしのOS X (Lion)は他のバグでテスト環境として不適切なのでFreeBSD環境の
人にテストお願いしたい。

あと、以前のメールで明確に書けていませんでしたが私が確認したのはSnow Leopardの
ソースなので、誰かが Snow Leopard以前でこの問題が発生しないことが確認しているなら
わたしがコードを誤読しています。コメント中に "Lion" と明記してあるので気になっています。

[kosaki (Motohiro KOSAKI)]

Issue #5075 has been updated by Akira Tanaka.

File recvmsg-msg_peek-freebsd.patch added

わたしのOS X (Lion)は他のバグでテスト環境として不適切なのでFreeBSD環境の
人にテストお願いしたい。

あと、以前のメールで明確に書けていませんでしたが私が確認したのはSnow Leopardの
ソースなので、誰かが Snow Leopard以前でこの問題が発生しないことが確認しているなら
わたしがコードを誤読しています。コメント中に "Lion" と明記してあるので気になっています。

[akr (Akira Tanaka)]

2011年7月23日8:40 KOSAKI Motohiro [e-mail]:

あと、以前のメールで明確に書けていませんでしたが私が確認したのはSnow Leopardの
ソースなので、誰かが Snow Leopard以前でこの問題が発生しないことが確認しているなら
わたしがコードを誤読しています。コメント中に "Lion" と明記してあるので気になっています。

そこはたしかに書き方を迷いました。
MacOS X Lion で問題を確認したのは nagachika さんです。

いまのところ誰も MacOS X Snow Leopard 以前での動作確認はしていないと思います。
(興味がある人がいたらなるせさんが提示したパッチを当てて test-all して、invalid が出てくるか
確認してみてください。)

[田中 哲][たなか あきら][Tanaka Akira]

[akr (Akira Tanaka)]

2011年7月23日8:40 KOSAKI Motohiro [e-mail]:

あと、以前のメールで明確に書けていませんでしたが私が確認したのはSnow Leopardの
ソースなので、誰かが Snow Leopard以前でこの問題が発生しないことが確認しているなら
わたしがコードを誤読しています。コメント中に "Lion" と明記してあるので気になっています。

そこはたしかに書き方を迷いました。
MacOS X Lion で問題を確認したのは nagachika さんです。

いまのところ誰も MacOS X Snow Leopard 以前での動作確認はしていないと思います。
(興味がある人がいたらなるせさんが提示したパッチを当てて test-all して、invalid が出てくるか
確認してみてください。)

[田中 哲][たなか あきら][Tanaka Akira]

[taca (Takahiro Kambe)]

In message [e-mail]
on Sat, 23 Jul 2011 01:37:35 +0900,
Tanaka Akira [e-mail] wrote:

Mac OS X と FreeBSD にて、存在しない fd を close してしまう問題について、
現在 r32598 で応急処置が施されていますが、根本的な原因について、
sys/kern/uipc_socket.c を見るに、
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c?rev=1.340.2.6.2.1;content-type=text%2Fplain;only_with_tag=RELENG_8_2_0_RELEASE

    * Process one or more MT_CONTROL mbufs present before any data mbufs
    * in the first mbuf chain on the socket buffer.  If MSG_PEEK, we
    * just copy the data; if !MSG_PEEK, we call into the protocol to
    * perform externalization (or freeing if controlp == NULL).

とあるので、recvmsg に MSG_PEEK を与えた場合は invalid なものが返ってくると思うのですが。

おぉ、素晴らしい。
このコメントは NetBSD current にもあります。FreeBSDから2008年4月14日に
取り込まれていて、リリースでは NetBSD 5には含まれています。

以上、just FYI.

--
神戸 隆博 / Takahiro Kambe

[taca (Takahiro Kambe)]

In message [e-mail]
on Sat, 23 Jul 2011 01:37:35 +0900,
Tanaka Akira [e-mail] wrote:

Mac OS X と FreeBSD にて、存在しない fd を close してしまう問題について、
現在 r32598 で応急処置が施されていますが、根本的な原因について、
sys/kern/uipc_socket.c を見るに、
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_socket.c?rev=1.340.2.6.2.1;content-type=text%2Fplain;only_with_tag=RELENG_8_2_0_RELEASE

    * Process one or more MT_CONTROL mbufs present before any data mbufs
    * in the first mbuf chain on the socket buffer.  If MSG_PEEK, we
    * just copy the data; if !MSG_PEEK, we call into the protocol to
    * perform externalization (or freeing if controlp == NULL).

とあるので、recvmsg に MSG_PEEK を与えた場合は invalid なものが返ってくると思うのですが。

おぉ、素晴らしい。
このコメントは NetBSD current にもあります。FreeBSDから2008年4月14日に
取り込まれていて、リリースでは NetBSD 5には含まれています。

以上、just FYI.

--
神戸 隆博 / Takahiro Kambe

[akr (Akira Tanaka)]

2011年7月23日9:26 Takahiro Kambe [e-mail]:

このコメントは NetBSD current にもあります。FreeBSDから2008年4月14日に
取り込まれていて、リリースでは NetBSD 5には含まれています。

では条件に defined(NetBSD) もつけるということで。
コンパイル時にテストしたい気もしますが。

本当は、カーネルはコンパイル時とは変わることもあるし、
動的に検査できるといいんですが。
fd を返さないときには、-1 にしてくれればいいのに。
(もしかして、事前にバッファを -1 で埋め尽くしておけばいい？)

[田中 哲][たなか あきら][Tanaka Akira]

[akr (Akira Tanaka)]

2011年7月23日9:26 Takahiro Kambe [e-mail]:

このコメントは NetBSD current にもあります。FreeBSDから2008年4月14日に
取り込まれていて、リリースでは NetBSD 5には含まれています。

では条件に defined(NetBSD) もつけるということで。
コンパイル時にテストしたい気もしますが。

本当は、カーネルはコンパイル時とは変わることもあるし、
動的に検査できるといいんですが。
fd を返さないときには、-1 にしてくれればいいのに。
(もしかして、事前にバッファを -1 で埋め尽くしておけばいい？)

[田中 哲][たなか あきら][Tanaka Akira]

[kosaki (Motohiro KOSAKI)]

では条件に defined(NetBSD) もつけるということで。
コンパイル時にテストしたい気もしますが。

本当は、カーネルはコンパイル時とは変わることもあるし、
動的に検査できるといいんですが。
fd を返さないときには、-1 にしてくれればいいのに。
(もしかして、事前にバッファを -1 で埋め尽くしておけばいい？)

今のところバージョン依存の要素は薄そうなので configureで判定してしまって
いいのではないでしょうか。
確認する気になれないけど Dragonflyとかもきっと同じ挙動を示すと思いますし

[kosaki (Motohiro KOSAKI)]

では条件に defined(NetBSD) もつけるということで。
コンパイル時にテストしたい気もしますが。

本当は、カーネルはコンパイル時とは変わることもあるし、
動的に検査できるといいんですが。
fd を返さないときには、-1 にしてくれればいいのに。
(もしかして、事前にバッファを -1 で埋め尽くしておけばいい？)

今のところバージョン依存の要素は薄そうなので configureで判定してしまって
いいのではないでしょうか。
確認する気になれないけど Dragonflyとかもきっと同じ挙動を示すと思いますし

[nagachika (Tomoyuki Chikanaga)]

わたしが確認したのは Snow Leopard でした。MSG_PEEK をつけた recvmsg() で不正な fd が取り出されるのを確認しました。

[akr (Akira Tanaka)]

• Status changed from Assigned to Closed

コンパイル時に挙動を検査するようにしました。

これで close とします。

[akr (Akira Tanaka)]

• File 5075-1.9.2.patch 5075-1.9.2.patch added
• Status changed from Closed to Assigned
• Assignee changed from akr (Akira Tanaka) to yugui (Yuki Sonoda)
• Target version changed from 1.9.3 to 1.9.2

1.9.2 用のパッチを作ったので添付します。

[naruse (Yui NARUSE)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport192
• Category changed from ext to ext
• Target version deleted (1.9.2)

[naruse (Yui NARUSE)]

• Status changed from Assigned to Rejected