Re: [PATCH] net: remove member 'max' of struct scm_fp_list

[yuan linyu]

On 五, 2017-02-10 at 10:25 -0500, David Miller wrote:
> From: yuan linyu 
> Date: Fri, 10 Feb 2017 20:11:13 +0800
> 
> > From: yuan linyu 
> > 
> > SCM_MAX_FD can fully replace it.
> > 
> > Signed-off-by: yuan linyu 
> 
> I don't think so:
> 
> > @@ -341,7 +332,6 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl)
> >   if (new_fpl) {
> >   for (i = 0; i < fpl->count; i++)
> >   get_file(fpl->fp[i]);
> > - new_fpl->max = new_fpl->count;
> >   new_fpl->user = get_uid(fpl->user);
> 
> It's not set the SCM_MAX_FD here, it's set to whatever fpl->count is.
> 
> In other words, your patch breaks things.
maybe it's not good to "SCM_MAX_FD can fully replace it".
actually 'max' field is useless.'count' field is enough.
 

Re: [PATCH] net: remove member 'max' of struct scm_fp_list

[David Miller]

From: yuan linyu 
Date: Fri, 10 Feb 2017 20:11:13 +0800

> From: yuan linyu 
> 
> SCM_MAX_FD can fully replace it.
> 
> Signed-off-by: yuan linyu 

I don't think so:

> @@ -341,7 +332,6 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl)
>   if (new_fpl) {
>   for (i = 0; i < fpl->count; i++)
>   get_file(fpl->fp[i]);
> - new_fpl->max = new_fpl->count;
>   new_fpl->user = get_uid(fpl->user);

It's not set the SCM_MAX_FD here, it's set to whatever fpl->count is.

In other words, your patch breaks things.

[PATCH] net: remove member 'max' of struct scm_fp_list

[yuan linyu]

From: yuan linyu 

SCM_MAX_FD can fully replace it.

Signed-off-by: yuan linyu 
---
 include/net/scm.h |  3 +--
 net/core/scm.c| 20 +---
 2 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/include/net/scm.h b/include/net/scm.h
index 59fa93c..1301227 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -19,8 +19,7 @@ struct scm_creds {
 };
 
 struct scm_fp_list {
-   short   count;
-   short   max;
+   unsigned intcount;
struct user_struct  *user;
struct file *fp[SCM_MAX_FD];
 };
diff --git a/net/core/scm.c b/net/core/scm.c
index b6d8368..53679517 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -69,15 +69,7 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct 
scm_fp_list **fplp)
int *fdp = (int*)CMSG_DATA(cmsg);
struct scm_fp_list *fpl = *fplp;
struct file **fpp;
-   int i, num;
-
-   num = (cmsg->cmsg_len - sizeof(struct cmsghdr))/sizeof(int);
-
-   if (num <= 0)
-   return 0;
-
-   if (num > SCM_MAX_FD)
-   return -EINVAL;
+   unsigned int i, num;
 
if (!fpl)
{
@@ -86,18 +78,17 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct 
scm_fp_list **fplp)
return -ENOMEM;
*fplp = fpl;
fpl->count = 0;
-   fpl->max = SCM_MAX_FD;
fpl->user = NULL;
}
-   fpp = >fp[fpl->count];
 
-   if (fpl->count + num > fpl->max)
+   num = (cmsg->cmsg_len - sizeof(struct cmsghdr))/sizeof(int);
+   if (fpl->count + num > SCM_MAX_FD)
return -EINVAL;
 
/*
 *  Verify the descriptors and increment the usage count.
 */
-
+   fpp = >fp[fpl->count];
for (i=0; i< num; i++)
{
int fd = fdp[i];
@@ -112,7 +103,7 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct 
scm_fp_list **fplp)
if (!fpl->user)
fpl->user = get_uid(current_user());
 
-   return num;
+   return 0;
 }
 
 void __scm_destroy(struct scm_cookie *scm)
@@ -341,7 +332,6 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl)
if (new_fpl) {
for (i = 0; i < fpl->count; i++)
get_file(fpl->fp[i]);
-   new_fpl->max = new_fpl->count;
new_fpl->user = get_uid(fpl->user);
}
return new_fpl;
-- 
2.7.4