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

Fixbug: fd may be 0 when BIO_get_fd return 0 #82

Open
wants to merge 1 commit into
base: master-babassl
Choose a base branch
from

Conversation

easycoderwei
Copy link

Fixbug.

PACKET pkt;
unsigned int version, type;
unsigned char buf[PEEK_HEADER_LENGTH];

ret = BIO_get_fd(s->rbio, &fd);
(void) BIO_get_fd(s->rbio, &fd);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当fd为0的时候,ret不会返回-1么?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

或者说fd没取到合法的值的时候,ret应该返回-1才对。还是说有可能ret和fd都是0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

或者说fd没取到合法的值的时候,ret应该返回-1才对。还是说有可能ret和fd都是0?

同意,如果有问题,也应该修改BIO_get_fd内部的逻辑

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我看了下BIO_get_fd的实现,如果该BIO没有初始化,应该返回-1,其余情况都是返回正常的fd值(即fd和ret值相等)。如果不是这种情况,那可能BIO_get_fd的实现就有问题了

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIO_get_fd内部实现没有问题。内部调用的sock_ctrl 返回值成功的话,ret = bio->num,bio->num 可能为0。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是这样,那么逻辑应该是:以BIO_get_fd()的返回值作为判断是否调用成功的依据,如果是-1则直接失败,大于等于0的都认为是成功:

ret = BIO_get_fd(s->rbio, &fd);
if (ret < 0) {
     SSLfatal_ntls(...);
}

改一行即可

Copy link
Author

@easycoderwei easycoderwei Sep 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是这样,那么逻辑应该是:以BIO_get_fd()的返回值作为判断是否调用成功的依据,如果是-1则直接失败,大于等于0的都认为是成功:

ret = BIO_get_fd(s->rbio, &fd);
if (ret < 0) {
     SSLfatal_ntls(...);
}

改一行即可

改一行是不行的。ret=0有两种场景:
1.成功:调用的sock_ctrl(b->method->ctrl)返回值成功的话,ret = bio->num,bio->num 可能为0。
2.失败:BIO_get_fd失败也有可能返回0

验证BIO_get_fd宏得到fd=0的场景,可以通过静态编译链接到nginx,然后nginx全局配置daemon off; 单次请求必现。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

正常启动进程fd=0会被占用,但是不能排除fd=0的场景。像nginx以非守护进程的方式启动就是例子。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret=0有两种场景:
1.成功:调用的sock_ctrl(b->method->ctrl)返回值成功的话,ret = bio->num,bio->num 可能为0。
2.失败:BIO_get_fd失败也有可能返回0

失败不会反回0,失败只会返回-1。所以返回0的时候,fd必为0。在成功的情况下,ret的值和fd的值是相同的。失败的时候ret为-1

Copy link
Author

@easycoderwei easycoderwei Sep 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ret=0有两种场景:
1.成功:调用的sock_ctrl(b->method->ctrl)返回值成功的话,ret = bio->num,bio->num 可能为0。
2.失败:BIO_get_fd失败也有可能返回0

失败不会反回0,失败只会返回-1。所以返回0的时候,fd必为0。在成功的情况下,ret的值和fd的值是相同的。失败的时候ret为-1

失败返回0的情况只有在bio==NULL的场景,在SSL_connection_is_ntls函数调用时确实是不会出现的。
但是从openssl中使用 BIO_get_fd 这个宏的地方,一般都不会依赖于返回值,而是依赖于输出参数fd的值,fd>=0就认为是合法的。

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants