diff mbox

apparmor: Fix harmless off by one in map_signal_num()

Message ID 20170901085225.dpuirzpj7aq3yyim@mwanda (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Carpenter Sept. 1, 2017, 8:52 a.m. UTC 
This patch has no effect on runtime.

The sig_map[] array has MAXMAPPED_SIG (35) members so my static checker
complains that the <= should be <.  But in this case it's not possible
for "sig" to be more than 31 because of the "else if (sig >= SIGRTMIN)"
condition since SIGRTMIN is 32.  The last three elements, 32-34, of
sig_map[] are empty so this code works as designed.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

John Johansen Sept. 25, 2017, 2:25 p.m. UTC | #1 
On 09/01/2017 04:52 AM, Dan Carpenter wrote:
> This patch has no effect on runtime.
> 
> The sig_map[] array has MAXMAPPED_SIG (35) members so my static checker
> complains that the <= should be <.  But in this case it's not possible
> for "sig" to be more than 31 because of the "else if (sig >= SIGRTMIN)"
> condition since SIGRTMIN is 32.  The last three elements, 32-34, of
> sig_map[] are empty so this code works as designed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> index 66fb9ede9447..5091c78062e4 100644
> --- a/security/apparmor/ipc.c
> +++ b/security/apparmor/ipc.c
> @@ -128,7 +128,7 @@ static inline int map_signal_num(int sig)
>  		return SIGUNKNOWN;
>  	else if (sig >= SIGRTMIN)
>  		return sig - SIGRTMIN + 128;	/* rt sigs mapped to 128 */
> -	else if (sig <= MAXMAPPED_SIG)
> +	else if (sig < MAXMAPPED_SIG)
>  		return sig_map[sig];
>  	return SIGUNKNOWN;
>  }
> 

Colin King beat you to this one, its in apparmor-next
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter Sept. 25, 2017, 3:09 p.m. UTC | #2 
On Mon, Sep 25, 2017 at 10:25:01AM -0400, John Johansen wrote:
> On 09/01/2017 04:52 AM, Dan Carpenter wrote:
> > This patch has no effect on runtime.
> > 
> > The sig_map[] array has MAXMAPPED_SIG (35) members so my static checker
> > complains that the <= should be <.  But in this case it's not possible
> > for "sig" to be more than 31 because of the "else if (sig >= SIGRTMIN)"
> > condition since SIGRTMIN is 32.  The last three elements, 32-34, of
> > sig_map[] are empty so this code works as designed.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> > index 66fb9ede9447..5091c78062e4 100644
> > --- a/security/apparmor/ipc.c
> > +++ b/security/apparmor/ipc.c
> > @@ -128,7 +128,7 @@ static inline int map_signal_num(int sig)
> >  		return SIGUNKNOWN;
> >  	else if (sig >= SIGRTMIN)
> >  		return sig - SIGRTMIN + 128;	/* rt sigs mapped to 128 */
> > -	else if (sig <= MAXMAPPED_SIG)
> > +	else if (sig < MAXMAPPED_SIG)
> >  		return sig_map[sig];
> >  	return SIGUNKNOWN;
> >  }
> > 
> 
> Colin King beat you to this one, its in apparmor-next

apparmor doesn't seem to be in linux-next at all.  Why is that?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Johansen Sept. 25, 2017, 4:27 p.m. UTC | #3 
On 09/25/2017 11:09 AM, Dan Carpenter wrote:
> On Mon, Sep 25, 2017 at 10:25:01AM -0400, John Johansen wrote:
>> On 09/01/2017 04:52 AM, Dan Carpenter wrote:
>>> This patch has no effect on runtime.
>>>
>>> The sig_map[] array has MAXMAPPED_SIG (35) members so my static checker
>>> complains that the <= should be <.  But in this case it's not possible
>>> for "sig" to be more than 31 because of the "else if (sig >= SIGRTMIN)"
>>> condition since SIGRTMIN is 32.  The last three elements, 32-34, of
>>> sig_map[] are empty so this code works as designed.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
>>> index 66fb9ede9447..5091c78062e4 100644
>>> --- a/security/apparmor/ipc.c
>>> +++ b/security/apparmor/ipc.c
>>> @@ -128,7 +128,7 @@ static inline int map_signal_num(int sig)
>>>  		return SIGUNKNOWN;
>>>  	else if (sig >= SIGRTMIN)
>>>  		return sig - SIGRTMIN + 128;	/* rt sigs mapped to 128 */
>>> -	else if (sig <= MAXMAPPED_SIG)
>>> +	else if (sig < MAXMAPPED_SIG)
>>>  		return sig_map[sig];
>>>  	return SIGUNKNOWN;
>>>  }
>>>
>>
>> Colin King beat you to this one, its in apparmor-next
> 
> apparmor doesn't seem to be in linux-next at all.  Why is that?
> 
hrmmm, apparmor is pulled in via security-next merges, but you are right
apparmor-next isn't being merged and we should set that up.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index 66fb9ede9447..5091c78062e4 100644
--- a/security/apparmor/ipc.c
+++ b/security/apparmor/ipc.c
@@ -128,7 +128,7 @@  static inline int map_signal_num(int sig)
 		return SIGUNKNOWN;
 	else if (sig >= SIGRTMIN)
 		return sig - SIGRTMIN + 128;	/* rt sigs mapped to 128 */
-	else if (sig <= MAXMAPPED_SIG)
+	else if (sig < MAXMAPPED_SIG)
 		return sig_map[sig];
 	return SIGUNKNOWN;
 }